You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/10/08 20:49:33 UTC

[GitHub] [tomcat] willmeck opened a new pull request #369: UDecoder does not properly convert when EncodedSolidusHandling is PASS_THROUGH and non-solidus encoded characters are present before the solidus

willmeck opened a new pull request #369:
URL: https://github.com/apache/tomcat/pull/369


   https://github.com/willmeck/tomcat-test-app - small test project showcasing the issue and fix
   
   When EncodedSolidusHandling is PASS_THROUGH, and other non-solidus encoded characters, such as encoded '+' (%2B) are present, the resultant string is incorrect. 
   Consider the string `abc%2Bde%2Ffg` 
   With PASS_THROUGH, it is expected that this would decode to `abc+de%2Ffg`
   However, looking at the UDecoder `convert()` code, this is what happens:
   
   buff contains `abc%2Bde%2Ffg`
   
   1. set idx to the first % character, `idx = 3`
   2. enter loop, setting j equal to idx, `idx=3` `j=3`
   3. buff[j] is `%` so enter else condition
   4. `j+=2` so `j=5 idx=3`
   5. set `res` equal to the char which is made up by j, j+1, and j+2
   6. res is not a `/`, so set buff[idx] = res (res is `+`) 
   
   buff now contains `abc+2Bde%2Ffg` and `j=5 idx=3`
   
   1. loop again so now `j=6 idx=4`
   2. buff[j] is `d`, so set buff[idx] = buff[j]
   3. loop again so now `j=7 idx=5'
   4. buff[j] is `e` so set buff[idx] = buff[j]
   
   buff now contains `abc+dede%2Ffg`
   
   1. loop again so now `j=8 idx=6`
   2. buff[j] is `%` so enter else condition
   3. `j+=2` so `j=10 idx=6`
   4. set `res` equal to the char which is made up by j, j+1, and j+2
   5. res is `/` so enter switch and go to case PASS_THROUGH
   6. `idx+=2` so `j=10 idx=8` (Note that at no point do you copy any chars from j to idx)
   
   buff now still contains `abc+dede%2Ffg`
   
   1. loop again so now `j=11 idx=9`
   2. buff[j] is `f` so set buff[idx] = buff[j]
   3. loop again so now `j=12 idx=10`
   4. buff[j] is `g` so set buff[idx] = buff[j]
   5. exit loop because reached length, adding 1 to idx, so `idx=11`
   6. set end at idx
   
   buff now contains `abc+dede%fg`
   
   The passthrough case statement expects j and idx to be equal, and that is not always the case, so we must explicitly copy those bytes. 


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on pull request #369: UDecoder does not properly convert when EncodedSolidusHandling is PASS_THROUGH and non-solidus encoded characters are present before the solidus

Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #369:
URL: https://github.com/apache/tomcat/pull/369#issuecomment-706113097


   Thanks for the PR. I've applied a variation of your proposed fix along with some additional unit tests to:
   - 10.0.x for 10.0.0-M10 onwards
   - 9.0.x for 9.0.40 onwards
   - 8.5.x for 8.5.60 onwards
   - 7.0.x for 7.0.107 onwards
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on pull request #369: UDecoder does not properly convert when EncodedSolidusHandling is PASS_THROUGH and non-solidus encoded characters are present before the solidus

Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #369:
URL: https://github.com/apache/tomcat/pull/369#issuecomment-706113097


   Thanks for the PR. I've applied a variation of your proposed fix along with some additional unit tests to:
   - 10.0.x for 10.0.0-M10 onwards
   - 9.0.x for 9.0.40 onwards
   - 8.5.x for 8.5.60 onwards
   - 7.0.x for 7.0.107 onwards
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf closed pull request #369: UDecoder does not properly convert when EncodedSolidusHandling is PASS_THROUGH and non-solidus encoded characters are present before the solidus

Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #369:
URL: https://github.com/apache/tomcat/pull/369


   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf closed pull request #369: UDecoder does not properly convert when EncodedSolidusHandling is PASS_THROUGH and non-solidus encoded characters are present before the solidus

Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #369:
URL: https://github.com/apache/tomcat/pull/369


   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org