Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On 12/03/2019 11:45, Jäkel, Guido wrote:
>> -Original Message-
>> From: Mark Thomas [mailto:ma...@apache.org]
>> Sent: Tuesday, March 12, 2019 12:51 AM
>> To: users@tomcat.apache.org
>> Subject: Re: Followup2: Changed behaviour of Tomcat
>> Deployment/Context/Lifecycle Manager concerning symbolic links
>
>> Looking at the code in ContextConfig.fixDocBase() it looks like it
>> should be possible to switch lines 585 and 587 to use getAbsolutePath()
>> without having too much impact on any performance improvements we may
>> want to consider. That should address the regression. @Guido can you
>> confirm that please?
>
>
> Using getAbsolutePath() instead of getPath() ...
>
> File file = new File(docBase);
> if (!file.isAbsolute()) {
> -docBase = (new File(appBase, docBase)).getCanonicalPath();
> +docBase = (new File(appBase, docBase)).getAbsolutePath();
> } else {
> -docBase = file.getCanonicalPath();
> +docBase = file.getAbsolutePath();
> }
>
> also still solve the issue for me.
Good to hear. That change will be in the next 9.0.x and 8.5.x releases.
Mark
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
>-Original Message-
>From: Johanes Soetanto [mailto:otnat...@gmail.com]
>Sent: Tuesday, March 12, 2019 9:03 PM
>To: Tomcat Users List
>Subject: Re: Followup2: Changed behaviour of Tomcat
>Deployment/Context/Lifecycle Manager concerning symbolic links
>
>Hi all,
>
>On Tue, 12 Mar. 2019, 9:45 pm Jäkel, Guido, wrote:
>
>Correct me if I'm wrong. The original reason of this discussion if the file
>extension does not end with war right? I don't see from test above that the
>links do not ends with war. Or is it because of trailing dot at the end?
Dear Johanes,
Yes, that sufficient because internal it's checked by endWith('.war'). In my
case, the filename ends with a version and a deployment node descriptor
appended. But you may append whatever you want and/or even left out the string
"war".
Guido
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Hi all,
On Tue, 12 Mar. 2019, 9:45 pm Jäkel, Guido, wrote:
>-Original Message-
>From: Mark Thomas [mailto:ma...@apache.org]
>Sent: Tuesday, March 12, 2019 12:51 AM
>To: users@tomcat.apache.org
>Subject: Re: Followup2: Changed behaviour of Tomcat
Deployment/Context/Lifecycle Manager concerning symbolic links
>Looking at the code in ContextConfig.fixDocBase() it looks like it
>should be possible to switch lines 585 and 587 to use getAbsolutePath()
>without having too much impact on any performance improvements we may
>want to consider. That should address the regression. @Guido can you
>confirm that please?
Using getAbsolutePath() instead of getPath() ...
File file = new File(docBase);
if (!file.isAbsolute()) {
-docBase = (new File(appBase,
docBase)).getCanonicalPath();
+docBase = (new File(appBase,
docBase)).getAbsolutePath();
} else {
-docBase = file.getCanonicalPath();
+docBase = file.getAbsolutePath();
}
also still solve the issue for me.
Here my minimal test set:
* At some auto-deploying location, create the empty WAR 'dst.war.'
(You may use the following, if you like)
# cat /opt/bin/mkemptyzip
#!/bin/bash
#
# 20180516/gj
if [ -z "$1" ]; then
cat >&2 <<-EOT
syntax : $0 ...
purpose: generate empty zip file
EOT
exit -1
fi
for FILE; do
[ -f "$FILE" ] && echo "file \"$FILE\" exists, skipping" &&
continue
echo -en
'\x50\x4b\x05\x06\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
>$FILE
done
* Set up some different styled symlinks to it:
root@testbcs0 /home/tomcatT-8/webapps/localhost # ll ???.*
lrwxrwxrwx 1 root root 41 Mar 12 12:32 abs.war ->
/data/srv/test/webapps/localhost/dst.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:33 chn.war. -> dst.war.
-rw-r--r-- 1 root root 22 Mar 12 12:32 dst.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:35 dsy.war -> chn.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:40 p#s.war -> chn.war.
lrwxrwxrwx 1 root root 8 Mar 12 11:51 lnk.war -> dst.war.
lrwxrwxrwx 1 root root 21 Mar 12 11:51 rel.war ->
../localhost/dst.war.
Correct me if I'm wrong. The original reason of this discussion if the file
extension does not end with war right? I don't see from test above that the
links do not ends with war. Or is it because of trailing dot at the end?
RE: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
>-Original Message-
>From: Mark Thomas [mailto:ma...@apache.org]
>Sent: Tuesday, March 12, 2019 12:51 AM
>To: users@tomcat.apache.org
>Subject: Re: Followup2: Changed behaviour of Tomcat
>Deployment/Context/Lifecycle Manager concerning symbolic links
>Looking at the code in ContextConfig.fixDocBase() it looks like it
>should be possible to switch lines 585 and 587 to use getAbsolutePath()
>without having too much impact on any performance improvements we may
>want to consider. That should address the regression. @Guido can you
>confirm that please?
Using getAbsolutePath() instead of getPath() ...
File file = new File(docBase);
if (!file.isAbsolute()) {
-docBase = (new File(appBase, docBase)).getCanonicalPath();
+docBase = (new File(appBase, docBase)).getAbsolutePath();
} else {
-docBase = file.getCanonicalPath();
+docBase = file.getAbsolutePath();
}
also still solve the issue for me.
Here my minimal test set:
* At some auto-deploying location, create the empty WAR 'dst.war.'
(You may use the following, if you like)
# cat /opt/bin/mkemptyzip
#!/bin/bash
#
# 20180516/gj
if [ -z "$1" ]; then
cat >&2 <<-EOT
syntax : $0 ...
purpose: generate empty zip file
EOT
exit -1
fi
for FILE; do
[ -f "$FILE" ] && echo "file \"$FILE\" exists, skipping" && continue
echo -en
'\x50\x4b\x05\x06\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
>$FILE
done
* Set up some different styled symlinks to it:
root@testbcs0 /home/tomcatT-8/webapps/localhost # ll ???.*
lrwxrwxrwx 1 root root 41 Mar 12 12:32 abs.war ->
/data/srv/test/webapps/localhost/dst.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:33 chn.war. -> dst.war.
-rw-r--r-- 1 root root 22 Mar 12 12:32 dst.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:35 dsy.war -> chn.war.
lrwxrwxrwx 1 root root 8 Mar 12 12:40 p#s.war -> chn.war.
lrwxrwxrwx 1 root root 8 Mar 12 11:51 lnk.war -> dst.war.
lrwxrwxrwx 1 root root 21 Mar 12 11:51 rel.war -> ../localhost/dst.war.
This will proper deploy the Contexts named '/abs' '/dsy' '/p/s' '/lnk' and
'/rel' . All this breaks while using getCanonical, but works with getPath() or
getAbsolutePath(). Notice also, that the "final destination" is still proper
watched, i.e. touching the 'dst.war.' will redeploy all contexts.
>I can run the unit tests and if they pass and the correction of the
>regression is confirmed it should be possible to get the fix into the
>next set of releases.
>
>The next release is almost certainly too soon for completing the
>performance review. That is probably going to need to wait until the
>following set of releases.
To remove the regression, IMHO you fist should revert the change for the next
release and do your wider code cleaning and performance after that.
with greetings
Guido
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On 11/03/2019 21:19, Michael Osipov wrote: > Am 2019-03-11 um 09:03 schrieb Rainer Jung: >> I think Mark refers to this one: >> >> https://marc.info/?l=tomcat-dev=153856675022101=2 > > Thanks Rainer. > > So this is a fix for another issue which clearly causes a regression. Sort of. A failure was reported when running some of the unit tests on Windows. That was fixed in this commit: https://github.com/apache/tomcat/commit/62c04ea42f115533437bccaa7873416abecc8c6e#diff-717001e0451788fe9f26d1176a4fff54 That fix had the potential to hide future case-sensitivity regressions so it was re-worked in: https://github.com/apache/tomcat/commit/db71c925106915581f1b60b0fda9c352fcdd9138#diff-717001e0451788fe9f26d1176a4fff54 While investigating the failures above it was noticed that ContextConfig was inconsistent. If the user supplied an absolute value for the docBase, the canonical version of that path was set as the docBase on the Context. If the user supplied a relative value for the docBase, the value was passed as is. There is no good reason that I can see for that inconsistency and potential (unproven) for things to break if the docBase contained sequences like "../..". Hence: https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 There are actually 3 different options for making those calls consistent: - getPath() - getAbsolutePath() - getCanonicalPath() getAbsolutePath() wasn't considered as it wasn't one of the options already in use and the preference was to keep change to a minimum. The value (part of it anyway) ends up stored in the Context docBase and the general expectation in the code is that that value is absolute / canonical hence getCanonicalPath() was chosen. > Looking into RFC 8089 (proposed), E.2 talks about canonicalization of > driver letters. The behaviour described there is consistent with observed behaviour with current Microsoft operating systems. Based on experience, I'd rather ask the OS what that it considers to the canonical path than rely on an RFC. > This needs to be taken care of solving both issues. > > @Guido, please create a BZ issue with a minimal test case. It is, and it isn't, quite that simple. I've spent a chunk of time today experimenting with this code to see what the impacts are, if any, of reverting to using getPath() here. On the plus side, I haven't been able to find a combination of code and configuration that breaks with either getPath() or getCanonicalPath(). I haven't tested it but I'm confident that if both those methods are OK then getAbsolutePath() will be OK as well. What I have also noticed is what seems like a large number of calls to getAbsolutePath() and getCanonicalPath(). Those are relatively expensive calls and I am wondering if there is scope for some performance improvement here. That needs a little more research as I was doing a lot of restarts and not paying attention to whether those calls were during restart or during request processing. Reducing calls during request processing will have a bigger performance improvement. Where things get interesting is, if there is scope to reduce the number of calls to getAbsolutePath() and/or getCanonicalPath(), to what extent do those improvements depend on the current code remaining as is? Looking at the code in ContextConfig.fixDocBase() it looks like it should be possible to switch lines 585 and 587 to use getAbsolutePath() without having too much impact on any performance improvements we may want to consider. That should address the regression. @Guido can you confirm that please? I can run the unit tests and if they pass and the correction of the regression is confirmed it should be possible to get the fix into the next set of releases. The next release is almost certainly too soon for completing the performance review. That is probably going to need to wait until the following set of releases. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Am 2019-03-11 um 09:03 schrieb Rainer Jung: Am 11.03.2019 um 08:09 schrieb Michael Osipov: Am 2019-03-10 um 22:29 schrieb Mark Thomas: On 10/03/2019 20:54, Michael Osipov wrote: Am 2019-03-10 um 12:16 schrieb Mark Thomas: On 10/03/2019 09:08, Guido Jäkel wrote: Dear John, Hi Rainer, Thank you for your hints. I leaned to used this features on Github locate the commit - it's https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 and it was committed by Mark Thomas. I don't know about the Tomcat project policies, but IMHO in the commit comment there was not any real hit for the motivation for the change or any reference to an issue ticket or a pull request. There's just one sentence for the changelog: Ensure that a canonical path is always used for the docBase of a Context to ensure consistent behaviour. (markt) But I can't get any idea from that what the author (Mark?) want to say with the terms "ensure" and "consistent". And it's classified as a "fix", but up to now I was not able to find the use case that is said to be fixed with this. From my point of view, the change lead to something what might be termed with "inconsistent", because now the link name is used as docBase, but the link destination is used to decide concrete aspects of Context loading. @Rainer: I familiarize me with the blame/history feature and have located the commit with this. But now, please tell me how to object against this change? Should I prepare a Git pull request against the master repository? Should I open an Issue somewhere? And how to locate the discussion that lead to this change? This should be tied to prevent flapping and respect and arrange with the motivation there. Changes aren't made on a whim. It is recommended that you investigate why a change was made before objecting to it. When a commit message in isolation appears to be missing context then that context can normally be found on the dev@ list. The 24 hours of dev@ traffic leading up to this commit should provide all the necessary background. There aren't any. I see the commit mail in Thunderbird, but no discussion on the change: Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> So, what now? The context is there on the dev@ list in the 24 hours leading up to that commit. Sorry, I seem to be blind. Can you point me to the discussion? I can't find anything in my tomcat-dev folder. I think Mark refers to this one: https://marc.info/?l=tomcat-dev=153856675022101=2 Thanks Rainer. So this is a fix for another issue which clearly causes a regression. Looking into RFC 8089 (proposed), E.2 talks about canonicalization of driver letters. This needs to be taken care of solving both issues. @Guido, please create a BZ issue with a minimal test case. Michael - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
>-Original Message- >From: Rainer Jung [mailto:rainer.j...@kippdata.de] >Sent: Monday, March 11, 2019 9:03 AM >To: Tomcat Users List ; Michael Osipov > >Subject: Re: Followup2: Changed behaviour of Tomcat >Deployment/Context/Lifecycle Manager concerning symbolic links > >Am 11.03.2019 um 08:09 schrieb Michael Osipov: >> Am 2019-03-10 um 22:29 schrieb Mark Thomas: >>> On 10/03/2019 20:54, Michael Osipov wrote: >>>> Am 2019-03-10 um 12:16 schrieb Mark Thomas: >>>>> On 10/03/2019 09:08, Guido Jäkel wrote: >>>>>> Dear John, Hi Rainer, >>>>>> >>>>>> Thank you for your hints. I leaned to used this features on Github >>>>>> locate the commit - it's >>>>>> >>>>>> >>>>>> https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 >>>>>> >>>>>> >>>>>> >>>>>> and it was committed by Mark Thomas. I don't know about the Tomcat >>>>>> project policies, but IMHO in the commit comment there was not any >>>>>> real hit for the motivation for the change or any reference to an >>>>>> issue ticket or a pull request. There's just one sentence for the >>>>>> changelog: >>>>>> >>>>>> Ensure that a canonical path is always used for the docBase >>>>>> of a Context >>>>>> to ensure consistent behaviour. (markt) >>>>>> >>>>>> But I can't get any idea from that what the author (Mark?) want to >>>>>> say with the terms "ensure" and "consistent". And it's classified as >>>>>> a "fix", but up to now I was not able to find the use case that is >>>>>> said to be fixed with this. >>>>>> >>>>>> From my point of view, the change lead to something what might be >>>>>> termed with "inconsistent", because now the link name is used as >>>>>> docBase, but the link destination is used to decide concrete aspects >>>>>> of Context loading. >>>>>> >>>>>> >>>>>> >>>>>> @Rainer: I familiarize me with the blame/history feature and have >>>>>> located the commit with this. But now, please tell me how to object >>>>>> against this change? Should I prepare a Git pull request against the >>>>>> master repository? Should I open an Issue somewhere? And how to >>>>>> locate the discussion that lead to this change? This should be tied >>>>>> to prevent flapping and respect and arrange with the motivation there. >>>>> >>>>> Changes aren't made on a whim. It is recommended that you investigate >>>>> why a change was made before objecting to it. >>>>> >>>>> When a commit message in isolation appears to be missing context then >>>>> that context can normally be found on the dev@ list. The 24 hours of >>>>> dev@ traffic leading up to this commit should provide all the necessary >>>>> background. >>>> >>>> There aren't any. I see the commit mail in Thunderbird, but no >>>> discussion on the change: >>>> >>>> Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> >>>> >>>> So, what now? >>> >>> The context is there on the dev@ list in the 24 hours leading up to that >>> commit. >> >> Sorry, I seem to be blind. Can you point me to the discussion? I can't >> find anything in my tomcat-dev folder. > >I think Mark refers to this one: > >https://marc.info/?l=tomcat-dev=153856675022101=2 > >Regards, > >Rainer Dear Rainer, Thank you for scanning the mail archive! This states that getCanonicalPath() was used because -- instead of getPath() -- is case insensitive (on Windows!). And the change was made to fix typo problems in a testcase (http://svn.apache.org/viewvc?rev=1842657=rev ) Well - using getCanonicalPath() might solve a typo case issue, but unfortunately this introduce another semantic. Guido
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Am 11.03.2019 um 08:09 schrieb Michael Osipov: Am 2019-03-10 um 22:29 schrieb Mark Thomas: On 10/03/2019 20:54, Michael Osipov wrote: Am 2019-03-10 um 12:16 schrieb Mark Thomas: On 10/03/2019 09:08, Guido Jäkel wrote: Dear John, Hi Rainer, Thank you for your hints. I leaned to used this features on Github locate the commit - it's https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 and it was committed by Mark Thomas. I don't know about the Tomcat project policies, but IMHO in the commit comment there was not any real hit for the motivation for the change or any reference to an issue ticket or a pull request. There's just one sentence for the changelog: Ensure that a canonical path is always used for the docBase of a Context to ensure consistent behaviour. (markt) But I can't get any idea from that what the author (Mark?) want to say with the terms "ensure" and "consistent". And it's classified as a "fix", but up to now I was not able to find the use case that is said to be fixed with this. From my point of view, the change lead to something what might be termed with "inconsistent", because now the link name is used as docBase, but the link destination is used to decide concrete aspects of Context loading. @Rainer: I familiarize me with the blame/history feature and have located the commit with this. But now, please tell me how to object against this change? Should I prepare a Git pull request against the master repository? Should I open an Issue somewhere? And how to locate the discussion that lead to this change? This should be tied to prevent flapping and respect and arrange with the motivation there. Changes aren't made on a whim. It is recommended that you investigate why a change was made before objecting to it. When a commit message in isolation appears to be missing context then that context can normally be found on the dev@ list. The 24 hours of dev@ traffic leading up to this commit should provide all the necessary background. There aren't any. I see the commit mail in Thunderbird, but no discussion on the change: Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> So, what now? The context is there on the dev@ list in the 24 hours leading up to that commit. Sorry, I seem to be blind. Can you point me to the discussion? I can't find anything in my tomcat-dev folder. I think Mark refers to this one: https://marc.info/?l=tomcat-dev=153856675022101=2 Regards, Rainer - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Am 2019-03-10 um 22:29 schrieb Mark Thomas: On 10/03/2019 20:54, Michael Osipov wrote: Am 2019-03-10 um 12:16 schrieb Mark Thomas: On 10/03/2019 09:08, Guido Jäkel wrote: Dear John, Hi Rainer, Thank you for your hints. I leaned to used this features on Github locate the commit - it's https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 and it was committed by Mark Thomas. I don't know about the Tomcat project policies, but IMHO in the commit comment there was not any real hit for the motivation for the change or any reference to an issue ticket or a pull request. There's just one sentence for the changelog: Ensure that a canonical path is always used for the docBase of a Context to ensure consistent behaviour. (markt) But I can't get any idea from that what the author (Mark?) want to say with the terms "ensure" and "consistent". And it's classified as a "fix", but up to now I was not able to find the use case that is said to be fixed with this. From my point of view, the change lead to something what might be termed with "inconsistent", because now the link name is used as docBase, but the link destination is used to decide concrete aspects of Context loading. @Rainer: I familiarize me with the blame/history feature and have located the commit with this. But now, please tell me how to object against this change? Should I prepare a Git pull request against the master repository? Should I open an Issue somewhere? And how to locate the discussion that lead to this change? This should be tied to prevent flapping and respect and arrange with the motivation there. Changes aren't made on a whim. It is recommended that you investigate why a change was made before objecting to it. When a commit message in isolation appears to be missing context then that context can normally be found on the dev@ list. The 24 hours of dev@ traffic leading up to this commit should provide all the necessary background. There aren't any. I see the commit mail in Thunderbird, but no discussion on the change: Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> So, what now? The context is there on the dev@ list in the 24 hours leading up to that commit. Sorry, I seem to be blind. Can you point me to the discussion? I can't find anything in my tomcat-dev folder. Michael - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On 10/03/2019 20:54, Michael Osipov wrote: > Am 2019-03-10 um 12:16 schrieb Mark Thomas: >> On 10/03/2019 09:08, Guido Jäkel wrote: >>> Dear John, Hi Rainer, >>> >>> Thank you for your hints. I leaned to used this features on Github >>> locate the commit - it's >>> >>> >>> https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 >>> >>> >>> and it was committed by Mark Thomas. I don't know about the Tomcat >>> project policies, but IMHO in the commit comment there was not any >>> real hit for the motivation for the change or any reference to an >>> issue ticket or a pull request. There's just one sentence for the >>> changelog: >>> >>> Ensure that a canonical path is always used for the docBase >>> of a Context >>> to ensure consistent behaviour. (markt) >>> >>> But I can't get any idea from that what the author (Mark?) want to >>> say with the terms "ensure" and "consistent". And it's classified as >>> a "fix", but up to now I was not able to find the use case that is >>> said to be fixed with this. >>> >>> From my point of view, the change lead to something what might be >>> termed with "inconsistent", because now the link name is used as >>> docBase, but the link destination is used to decide concrete aspects >>> of Context loading. >>> >>> >>> >>> @Rainer: I familiarize me with the blame/history feature and have >>> located the commit with this. But now, please tell me how to object >>> against this change? Should I prepare a Git pull request against the >>> master repository? Should I open an Issue somewhere? And how to >>> locate the discussion that lead to this change? This should be tied >>> to prevent flapping and respect and arrange with the motivation there. >> >> Changes aren't made on a whim. It is recommended that you investigate >> why a change was made before objecting to it. >> >> When a commit message in isolation appears to be missing context then >> that context can normally be found on the dev@ list. The 24 hours of >> dev@ traffic leading up to this commit should provide all the necessary >> background. > > There aren't any. I see the commit mail in Thunderbird, but no > discussion on the change: > > Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> > > So, what now? The context is there on the dev@ list in the 24 hours leading up to that commit. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Am 2019-03-10 um 12:16 schrieb Mark Thomas: On 10/03/2019 09:08, Guido Jäkel wrote: Dear John, Hi Rainer, Thank you for your hints. I leaned to used this features on Github locate the commit - it's https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 and it was committed by Mark Thomas. I don't know about the Tomcat project policies, but IMHO in the commit comment there was not any real hit for the motivation for the change or any reference to an issue ticket or a pull request. There's just one sentence for the changelog: Ensure that a canonical path is always used for the docBase of a Context to ensure consistent behaviour. (markt) But I can't get any idea from that what the author (Mark?) want to say with the terms "ensure" and "consistent". And it's classified as a "fix", but up to now I was not able to find the use case that is said to be fixed with this. From my point of view, the change lead to something what might be termed with "inconsistent", because now the link name is used as docBase, but the link destination is used to decide concrete aspects of Context loading. @Rainer: I familiarize me with the blame/history feature and have located the commit with this. But now, please tell me how to object against this change? Should I prepare a Git pull request against the master repository? Should I open an Issue somewhere? And how to locate the discussion that lead to this change? This should be tied to prevent flapping and respect and arrange with the motivation there. Changes aren't made on a whim. It is recommended that you investigate why a change was made before objecting to it. When a commit message in isolation appears to be missing context then that context can normally be found on the dev@ list. The 24 hours of dev@ traffic leading up to this commit should provide all the necessary background. There aren't any. I see the commit mail in Thunderbird, but no discussion on the change: Message-Id: <20181003111609.0b0143a0...@svn01-us-west.apache.org> So, what now? - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On 10/03/2019 09:08, Guido Jäkel wrote: > Dear John, Hi Rainer, > > Thank you for your hints. I leaned to used this features on Github locate the > commit - it's > > > https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6 > > and it was committed by Mark Thomas. I don't know about the Tomcat project > policies, but IMHO in the commit comment there was not any real hit for the > motivation for the change or any reference to an issue ticket or a pull > request. There's just one sentence for the changelog: > > Ensure that a canonical path is always used for the docBase of a > Context > to ensure consistent behaviour. (markt) > > But I can't get any idea from that what the author (Mark?) want to say with > the terms "ensure" and "consistent". And it's classified as a "fix", but up > to now I was not able to find the use case that is said to be fixed with this. > > From my point of view, the change lead to something what might be termed with > "inconsistent", because now the link name is used as docBase, but the link > destination is used to decide concrete aspects of Context loading. > > > > @Rainer: I familiarize me with the blame/history feature and have located the > commit with this. But now, please tell me how to object against this change? > Should I prepare a Git pull request against the master repository? Should I > open an Issue somewhere? And how to locate the discussion that lead to this > change? This should be tied to prevent flapping and respect and arrange with > the motivation there. Changes aren't made on a whim. It is recommended that you investigate why a change was made before objecting to it. When a commit message in isolation appears to be missing context then that context can normally be found on the dev@ list. The 24 hours of dev@ traffic leading up to this commit should provide all the necessary background. Archives for dev@ are listed here: http://tomcat.apache.org/lists.html#tomcat-dev Personally, I favour MarkMail's UI but that is very much a personal choice. All the archives have the same messages. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Dear John, Hi Rainer,
Thank you for your hints. I leaned to used this features on Github locate the
commit - it's
https://github.com/apache/tomcat/commit/fd2abbb525660a9968694afd99a58f8c22cb54c6
and it was committed by Mark Thomas. I don't know about the Tomcat project
policies, but IMHO in the commit comment there was not any real hit for the
motivation for the change or any reference to an issue ticket or a pull
request. There's just one sentence for the changelog:
Ensure that a canonical path is always used for the docBase of a Context
to ensure consistent behaviour. (markt)
But I can't get any idea from that what the author (Mark?) want to say with the
terms "ensure" and "consistent". And it's classified as a "fix", but up to now
I was not able to find the use case that is said to be fixed with this.
>From my point of view, the change lead to something what might be termed with
>"inconsistent", because now the link name is used as docBase, but the link
>destination is used to decide concrete aspects of Context loading.
@Rainer: I familiarize me with the blame/history feature and have located the
commit with this. But now, please tell me how to object against this change?
Should I prepare a Git pull request against the master repository? Should I
open an Issue somewhere? And how to locate the discussion that lead to this
change? This should be tied to prevent flapping and respect and arrange with
the motivation there.
@John: And thank *you* for your curiosity, I just wand to satisfy this but not
self-adulate with the explanation:
As mentioned, I'm not working as a Developer but as an Unix operations system
architect. I have worked out my first programs on TRS80 and PET and from this I
have a wide background what I would code if to have to write something to
implement a task. And because I work at the operating, it's a big part of my
daily work to "reverse engineer" the issues "stashed" by my coworkers at the
dev department from the symptoms in an -- in the eyes of a typical developer --
unusual way.
I'm also the builder of my own architecture and i use Gentoo Linux for this.
Here, Tomcat is build from the sources and in addition, Gentoo offers the
(unique?) feature to free choose a concrete version of the upstream sources and
(if the ebuilds are prepared for) even to install versions in parallel. This is
not the case for Tomcat, but one may nevertheless enroll different sources for
building and compiling in parallel.
>From the behaviour and stack trace, I got the package names and the area of
>the code tree. I start to read the source code and did some 'grep -r foo *'
>and 'diff -r tomcat-5.8.{23,37}/' on the code tree. I intentional focused on
>small changes first and after some minutes I found the "perfect match": A
>single line changed from getPath() to getCanicalPath(). After a quick look on
>the surrounding code (using vi) on an abstract level (i.e. used naming,
>visible intention of the code) I was sure to bet on this. Then I revert the
>change, let the whole compile, package an install and prove it by an
>rollout-out the problematic target environment (one of my LX-Containers). And
>as expected, the issue vanished and all installed application deploy in the
>same way as before.
greetings
Guido
On 09.03.19 15:02, John Dale wrote:
> Nice investigative work, Guido.
>
> Curious, are you debugging the source code? Downloading any nightly builds?
>
> If you're connected to the repo somehow you could get users named on
> the commit logs and read commit messages?
>
> Again - nice work!
>
> Sincerely,
>
> John
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Hi Guido, Am 09.03.2019 um 10:09 schrieb Guido Jäkel: Dear Mark, thank you for comments and hints. I would say I have a wide knowledge about hard and software. But as I'm not working as a software developer, I'm not familiar with a lot of things in deep. I also don't have key-ready workbenches or buildchains. But I'll try to locate the corresponding commit using web access to the git. May I also contact you afterwards for further steps? Should I try to open an issue on the git or should I start a discussion in the Tomcat developer mailing list? To add small hints: the project is available on Github, which provides an easy web interface for basic code archeology. E.g. the class you metioned can be viewed at https://github.com/apache/tomcat/blob/master/java/org/apache/catalina/startup/ContextConfig.java and that page contains buttons for "Blame" - which shows when and in which commit each line was changes last, and also "History" which shows the list of changes applied to that file. The above link if for master (TC 9), but analogous pages exist for each branch, e.g. https://github.com/apache/tomcat/blob/8.5.x/java/org/apache/catalina/startup/ContextConfig.java Regards, Rainer On 08.03.19 21:58, Mark Thomas wrote: On 08/03/2019 11:59, Jäkel, Guido wrote: Good news! I reverted the change and this solve my issue at once, i.e. all former installed applications will start up as expected. So, please what was the reason or intention here to shift from getPath() to getCanonicalPath() in case of a link (detected by !file.isAbsolute() )? What's the motivation to "fully expand" the path here at Java level instead of delegating this to the underlying OS? Tomcat is an open source project. git (and svn that we used until recently) provides a feature that lets you identify the most recent commit associated with any line of code. Every commit includes a log message. That is usually where you'd find an explanation for why a commit was made. Have you tried looking? Mark greetings Guido (I'm going to check this out right now) May somebody point me to a ticket for the commit of this change and/or an issue ticket leading to this change? I want to know the motivation for this change and I want to please to find a solution to keep the old behavior. Because in my eyes, the current is inconsistent: For the context naming and so on, the well-known behavior is kept -- the context is named by the naming of the link itself and not of it's destination. And therefore, this should also hold for all other aspects - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Nice investigative work, Guido.
Curious, are you debugging the source code? Downloading any nightly builds?
If you're connected to the repo somehow you could get users named on
the commit logs and read commit messages?
Again - nice work!
Sincerely,
John
On 3/8/19, Jäkel, Guido wrote:
> Good news!
>
> I reverted the change and this solve my issue at once, i.e. all former
> installed applications will start up as expected.
>
> So, please what was the reason or intention here to shift from getPath() to
> getCanonicalPath() in case of a link (detected by !file.isAbsolute() )?
> What's the motivation to "fully expand" the path here at Java level instead
> of delegating this to the underlying OS?
>
> greetings
>
> Guido
>
>>-Original Message-
>>From: Jäkel, Guido [mailto:g.jae...@dnb.de]
>>Sent: Friday, March 08, 2019 11:39 AM
>>To: 'Tomcat Users List'
>>Subject: Followup: Changed behaviour of Tomcat Deployment/Context/Lifecycle
>> Manager concerning symbolic links
>>
>> [...]
>>
>>And just from the names of the used methods, I wonder that the root cause
>> is the following change
>>
>>
>> diff -r -u
>> /var/tmp/portage/www-servers/tomcat-8.5.23/work/apache-tomcat-8.5.23-
>>src/java/org/apache/catalina/startup/ContextConfig.java
>> /var/tmp/portage/www-servers/tomcat-8.5.37/work/apache-tomcat-8.5.37-
>>src/java/org/apache/catalina/startup/ContextConfig.java
>>
>> [...]
>> @@ -589,7 +583,7 @@
>>
>> File file = new File(docBase);
>> if (!file.isAbsolute()) {
>>-docBase = (new File(appBase, docBase)).getPath();
>>+docBase = (new File(appBase, docBase)).getCanonicalPath();
>> } else {
>> docBase = file.getCanonicalPath();
>> }
>> [...]
>>
>>(I'm going to check this out right now)
>>
>>May somebody point me to a ticket for the commit of this change and/or an
>> issue ticket leading to this change? I want to know
>>the motivation for this change and I want to please to find a solution to
>> keep the old behavior. Because in my eyes, the current
>>is inconsistent: For the context naming and so on, the well-known behavior
>> is kept -- the context is named by the naming of the
>>link itself and not of it's destination. And therefore, this should also
>> hold for all other aspects
>>
>>
>>greetings
>>
>>Guido
>
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On March 9, 2019 9:09:42 AM UTC, "Guido Jäkel" wrote: >Dear Mark, > >thank you for comments and hints. I would say I have a wide knowledge >about hard and software. But as I'm not working as a software >developer, I'm not familiar with a lot of things in deep. I also don't >have key-ready workbenches or buildchains. But I'll try to locate the >corresponding commit using web access to the git. May I also contact >you afterwards for further steps? Should I try to open an issue on the >git or should I start a discussion in the Tomcat developer mailing >list? > >Guido > >On 08.03.19 21:58, Mark Thomas wrote: >> On 08/03/2019 11:59, Jäkel, Guido wrote: >>> Good news! >>> >>> I reverted the change and this solve my issue at once, i.e. all >former installed applications will start up as expected. >>> >>> So, please what was the reason or intention here to shift from >getPath() to getCanonicalPath() in case of a link (detected by >!file.isAbsolute() )? What's the motivation to "fully expand" the path >here at Java level instead of delegating this to the underlying OS? >> >> Tomcat is an open source project. git (and svn that we used until >> recently) provides a feature that lets you identify the most recent >> commit associated with any line of code. Every commit includes a log >> message. That is usually where you'd find an explanation for why a >> commit was made. Have you tried looking? >> >> Mark >> >>> >>> greetings >>> >>> Guido >>> (I'm going to check this out right now) May somebody point me to a ticket for the commit of this change >and/or an issue ticket leading to this change? I want to know the motivation for this change and I want to please to find a >solution to keep the old behavior. Because in my eyes, the current is inconsistent: For the context naming and so on, the well-known >behavior is kept -- the context is named by the naming of the link itself and not of it's destination. And therefore, this should >also hold for all other aspects > >- >To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org >For additional commands, e-mail: users-h...@tomcat.apache.org This mailing list is the correct location should you have any follow-up questions. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Dear Mark, thank you for comments and hints. I would say I have a wide knowledge about hard and software. But as I'm not working as a software developer, I'm not familiar with a lot of things in deep. I also don't have key-ready workbenches or buildchains. But I'll try to locate the corresponding commit using web access to the git. May I also contact you afterwards for further steps? Should I try to open an issue on the git or should I start a discussion in the Tomcat developer mailing list? Guido On 08.03.19 21:58, Mark Thomas wrote: > On 08/03/2019 11:59, Jäkel, Guido wrote: >> Good news! >> >> I reverted the change and this solve my issue at once, i.e. all former >> installed applications will start up as expected. >> >> So, please what was the reason or intention here to shift from getPath() to >> getCanonicalPath() in case of a link (detected by !file.isAbsolute() )? >> What's the motivation to "fully expand" the path here at Java level instead >> of delegating this to the underlying OS? > > Tomcat is an open source project. git (and svn that we used until > recently) provides a feature that lets you identify the most recent > commit associated with any line of code. Every commit includes a log > message. That is usually where you'd find an explanation for why a > commit was made. Have you tried looking? > > Mark > >> >> greetings >> >> Guido >> >>> (I'm going to check this out right now) >>> >>> May somebody point me to a ticket for the commit of this change and/or an >>> issue ticket leading to this change? I want to know >>> the motivation for this change and I want to please to find a solution to >>> keep the old behavior. Because in my eyes, the current >>> is inconsistent: For the context naming and so on, the well-known behavior >>> is kept -- the context is named by the naming of the >>> link itself and not of it's destination. And therefore, this should also >>> hold for all other aspects >>> >>> - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
On 08/03/2019 11:59, Jäkel, Guido wrote:
> Good news!
>
> I reverted the change and this solve my issue at once, i.e. all former
> installed applications will start up as expected.
>
> So, please what was the reason or intention here to shift from getPath() to
> getCanonicalPath() in case of a link (detected by !file.isAbsolute() )?
> What's the motivation to "fully expand" the path here at Java level instead
> of delegating this to the underlying OS?
Tomcat is an open source project. git (and svn that we used until
recently) provides a feature that lets you identify the most recent
commit associated with any line of code. Every commit includes a log
message. That is usually where you'd find an explanation for why a
commit was made. Have you tried looking?
Mark
>
> greetings
>
> Guido
>
>> -Original Message-
>> From: Jäkel, Guido [mailto:g.jae...@dnb.de]
>> Sent: Friday, March 08, 2019 11:39 AM
>> To: 'Tomcat Users List'
>> Subject: Followup: Changed behaviour of Tomcat Deployment/Context/Lifecycle
>> Manager concerning symbolic links
>>
>> [...]
>>
>> And just from the names of the used methods, I wonder that the root cause is
>> the following change
>>
>>
>> diff -r -u
>> /var/tmp/portage/www-servers/tomcat-8.5.23/work/apache-tomcat-8.5.23-
>> src/java/org/apache/catalina/startup/ContextConfig.java
>> /var/tmp/portage/www-servers/tomcat-8.5.37/work/apache-tomcat-8.5.37-
>> src/java/org/apache/catalina/startup/ContextConfig.java
>>
>> [...]
>> @@ -589,7 +583,7 @@
>>
>> File file = new File(docBase);
>> if (!file.isAbsolute()) {
>> -docBase = (new File(appBase, docBase)).getPath();
>> +docBase = (new File(appBase, docBase)).getCanonicalPath();
>> } else {
>> docBase = file.getCanonicalPath();
>> }
>> [...]
>>
>> (I'm going to check this out right now)
>>
>> May somebody point me to a ticket for the commit of this change and/or an
>> issue ticket leading to this change? I want to know
>> the motivation for this change and I want to please to find a solution to
>> keep the old behavior. Because in my eyes, the current
>> is inconsistent: For the context naming and so on, the well-known behavior
>> is kept -- the context is named by the naming of the
>> link itself and not of it's destination. And therefore, this should also
>> hold for all other aspects
>>
>>
>> greetings
>>
>> Guido
>
> -
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Followup2: Changed behaviour of Tomcat Deployment/Context/Lifecycle Manager concerning symbolic links
Good news!
I reverted the change and this solve my issue at once, i.e. all former
installed applications will start up as expected.
So, please what was the reason or intention here to shift from getPath() to
getCanonicalPath() in case of a link (detected by !file.isAbsolute() )? What's
the motivation to "fully expand" the path here at Java level instead of
delegating this to the underlying OS?
greetings
Guido
>-Original Message-
>From: Jäkel, Guido [mailto:g.jae...@dnb.de]
>Sent: Friday, March 08, 2019 11:39 AM
>To: 'Tomcat Users List'
>Subject: Followup: Changed behaviour of Tomcat Deployment/Context/Lifecycle
>Manager concerning symbolic links
>
> [...]
>
>And just from the names of the used methods, I wonder that the root cause is
>the following change
>
>
> diff -r -u
> /var/tmp/portage/www-servers/tomcat-8.5.23/work/apache-tomcat-8.5.23-
>src/java/org/apache/catalina/startup/ContextConfig.java
>/var/tmp/portage/www-servers/tomcat-8.5.37/work/apache-tomcat-8.5.37-
>src/java/org/apache/catalina/startup/ContextConfig.java
>
> [...]
> @@ -589,7 +583,7 @@
>
> File file = new File(docBase);
> if (!file.isAbsolute()) {
>-docBase = (new File(appBase, docBase)).getPath();
>+docBase = (new File(appBase, docBase)).getCanonicalPath();
> } else {
> docBase = file.getCanonicalPath();
> }
> [...]
>
>(I'm going to check this out right now)
>
>May somebody point me to a ticket for the commit of this change and/or an
>issue ticket leading to this change? I want to know
>the motivation for this change and I want to please to find a solution to keep
>the old behavior. Because in my eyes, the current
>is inconsistent: For the context naming and so on, the well-known behavior is
>kept -- the context is named by the naming of the
>link itself and not of it's destination. And therefore, this should also hold
>for all other aspects
>
>
>greetings
>
>Guido
