[ 
https://issues.apache.org/jira/browse/LOG4J2-3621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17619756#comment-17619756
 ] 

Adwait Kumar Singh edited comment on LOG4J2-3621 at 10/20/22 5:59 PM:
----------------------------------------------------------------------

Also [~pkarwasz], regarding the property ordering, we seemed to have broken 
users who relied on {{priority of legacy properties < environment variables}}. 
We would be fixing it here, but we would be breaking anyone who took a 
dependency on the reverse i.e {{priority of legacy properties > environment 
variables}} in 2.19. 

So wanted to check what would be the release plan for this, would we release it 
in 2.20 or a minor version of 2.19. Also I think it would be a good idea to 
call out this behaviour in the docs, I can make changes for the same.


was (Author: adwsingh):
Also regarding the property ordering, we seemed to have broken users who relied 
on {{priority of legacy properties < environment variables}}. We would be 
fixing it here, but we would be breaking anyone who took a dependency on the 
reverse i.e {{priority of legacy properties > environment variables}} in 2.19. 

So wanted to check what would be the release plan for this, would we release it 
in 2.20 or a minor version of 2.19. Also I think it would be a good idea to 
call out this behaviour in the docs, I can make changes for the same.

> Log4J 2.19 breaks contract of order of loading of System Properties
> -------------------------------------------------------------------
>
>                 Key: LOG4J2-3621
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-3621
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: Configuration
>    Affects Versions: 2.19.0, 2.19.1
>            Reporter: Adwait Kumar Singh
>            Priority: Major
>
> [This change|https://github.com/apache/logging-log4j2/pull/742] broke one of 
> our systems on upgrading to 2.19.
> In our applications we had both LOG4J_CONFIGURATION_FILE environment variable 
> as well as log4j.configurationFile system property set.
> In version 2.17.2 "log4j.configurationFile" gets priority vs in 2.19 
> "LOG4J_CONFIGURATION_FILE" gets priority. This also breaks the contract 
> mentioned in the 
> [documentation|https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties].
> This is happening because of the normalization code here, 
> [https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L503-L526]
>  
> When we are trying to normalize, we are checking if the source contains the 
> normalKey. In case both log4j.configurationFile and LOG4J_CONFIGURATION_FILE 
> are present, the following sequence happens,
>  # log4j.configurationFile does not get inserted into the normalized map 
> because the normal key is log4j2.configurationFile which is not present in 
> the SystemPropertiesSource.
>  # Then when we hit the EnvironmentPropertiesSource, log4j.configurationFile 
> is normalized to LOG4J_CONFIGURATION_FILE and then an entry is made in the 
> normalized map with key = log4j.configurationFile, but value of 
> LOG4J_CONFIGURATION_FILE.
>  # During look up with first look into normalized map, so now we got the 
> wrong value (EnvironmentVariable instead of SystemProperty).
>  
> I am aware changing "log4j.configurationFile" to "log4j2.configurationFile" 
> can fix the issue, however this is clearly a backward incompatible change 
> which will require this change across a lot of consumers who want to upgrade 
> to log4j 2.19
> I can think of two ways to fix this,
>  # We make an entry into the normalized map with the actual key if the 
> normalized key is not present in the source, OR
>  # While fetching we prefer literal map over normalized map.
> Would like to know which of the approaches would be better, can raise a PR 
> accordingly.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to