Re: [PR] [JSPWIKI-1178] Address potential deadlock with JDK 21 Virtual Threads. (jspwiki)

[via GitHub]

arturobernalg commented on PR #307:
URL: https://github.com/apache/jspwiki/pull/307#issuecomment-1745343221

   > Hi,
   > 
   > interesting PR! :-) My only comment is that I'd extract the lock behaviour 
on a utility class something similar to (pseudocode, surely doesn't even 
compile):
   > 
   > ```java
   > public class Synchronizer {
   > 
   > public static < T > T synchronize( final ReentrantLock lock, final 
Supplier< T > supplier ) {
   > lock.lock();
   >try {
   >return supplier.get();
   >} finally {
   >lock.unlock();
   >}
   > }
   > 
   > }
   > ```
   > 
   > so code inside synchronized blocks can be passed as a lambda (also this 
method could be overloaded so it could also receive some Runnable, Function, 
etc.)
   > 
   > WDYT?
   > 
   > cheers, juan pablo
   
   HI @juanpablo-santos 
   
   I agree that encapsulating the lock behavior in a utility class like 
Synchronizer would offer several advantages, particularly in terms of code 
Readability and Reusability.
   
   However, this approach has limitations when it comes to working with 
condition variables and allowing for custom scenarios. Specifically, using a 
utility class for locking would make it challenging to implement more complex 
control flows that involve waiting for certain conditions to be met or 
signaling other threads to proceed.
   
   In essence, while the utility class would make the code cleaner for basic 
locking and unlocking, it might not be flexible enough to handle advanced 
locking scenarios that require the use of conditions.
   
   Best regards,


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@jspwiki.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Discussion on Fixing Relative URLs in Email Notifications (JSPWIKI-1056)

[Arturo Bernal]

Hi Team,

I hope this email finds you well. I am writing to open a discussion on the
issue JSPWIKI-1056 ,
which concerns the generation of relative URLs in email notifications sent
after user registration.

As some of you may know, the emails currently contain relative URLs due to
changes in JSPWIKI-1035 .
I have submitted a pull request (PR #311
) that aims to address this by
generating absolute URLs. The PR introduces utility methods in HttpUtil for
this purpose.

However, there are concerns about how this approach handles different
deployment scenarios, especially when JSPWiki installations are behind a
web server like Apache. The issue is that using HttpServletRequest to
generate the URL could expose internal URLs, which is not intended.

I would like to invite your thoughts on how best to tackle this issue. Some
options include:

   1. Checking for specific headers that might contain the "external"
   IP/domain.
   2. Introducing a new configuration option to set the base URL explicitly.

I look forward to your input on this matter. Your expertise and insights
would be invaluable in finding the most robust and flexible solution.

Best regards,

Arturo

Re: Discussion on Fixing Relative URLs in Email Notifications (JSPWIKI-1056)

[Arturo Bernal]

HI,

Maybe we should consider extending the URL construction logic to include
checks for the X-Forwarded-Server header, in addition to X-Forwarded-Host
and X-Forwarded-Proto. This would offer a more comprehensive way to
determine the original server and scheme, particularly in scenarios where
the application is behind a proxy.

What are your thoughts on this approach?

Arturo


On Tue, Oct 3, 2023 at 6:41 PM Arturo Bernal  wrote:

> Hi Team,
>
> I hope this email finds you well. I am writing to open a discussion on the
> issue JSPWIKI-1056 ,
> which concerns the generation of relative URLs in email notifications sent
> after user registration.
>
> As some of you may know, the emails currently contain relative URLs due to
> changes in JSPWIKI-1035
> . I have submitted a
> pull request (PR #311 ) that
> aims to address this by generating absolute URLs. The PR introduces utility
> methods in HttpUtil for this purpose.
>
> However, there are concerns about how this approach handles different
> deployment scenarios, especially when JSPWiki installations are behind a
> web server like Apache. The issue is that using HttpServletRequest to
> generate the URL could expose internal URLs, which is not intended.
>
> I would like to invite your thoughts on how best to tackle this issue.
> Some options include:
>
>1. Checking for specific headers that might contain the "external"
>IP/domain.
>2. Introducing a new configuration option to set the base URL
>explicitly.
>
> I look forward to your input on this matter. Your expertise and insights
> would be invaluable in finding the most robust and flexible solution.
>
> Best regards,
>
> Arturo
>