#663 ✓invalid
mateas

UrlDecoder for request path fails when parsing request

Reported by mateas | March 18th, 2011 @ 03:47 PM

When Play! decodes request path containing query string with encoded unicode characters such as %u00e4 play! fails with IllegalArgumentException:

java.lang.IllegalArgumentException: URLDecoder: Illegal hex characters in escape (%) pattern - For input string: "u0"

Framework version: 1.1.1

Reproduction steps: http://localhost:9000/anypath?myparam=someValue%u00e4includingUnico...

Details:
PlayHandlers.parseRequest(ChannelHandlerContext ctx, HttpRequest nettyRequest) decodes the full path including the query string which fails when query string contains "illigal" characters.

Solution:
Only decode the path excluding the query string (which Play! already does if query string is present)

The bug causes problems for url backward compatibility from a previous system.

Comments and changes to this ticket

  • Morten Kjetland

    Morten Kjetland April 24th, 2011 @ 10:40 PM

    • State changed from “new” to “invalid”
    • Assigned user set to “Morten Kjetland”

    I think the cause of this problem is that your url should have been URLEncoded in the first place:

    The the url would have looked like this:

    http://localhost:9000/anypath?myparam=someValue%%u00e4includingUnicodes%%u00e4gen
    

    And it would end up like the original url after it has been URLDecoded.

  • mateas

    mateas April 27th, 2011 @ 12:23 PM

    Yes, the values are not encoded using url encoding standards, I agree on that. Its encoded using some library with a non-standard implementation rejected by W3C described here http://en.wikipedia.org/wiki/Percent-encoding#Non-standard_implementations

    Although, as I wrote in the initial request this cases problems for backward compatibility with a previous system. It is not possible to change the original URLs as they are created in a legacy web app and are bookmarked by users and partners all over the web. The new web solution (our Play! app) require to be backward compatible with these URLs but Play makes this very hard since Play! crashes before our rewrite Play! plugin can handle the request.

    The submitted patch did not change the outcome of the request decoding. It simply made the query string parameters not to be included when decoding the request which was the original case.

  • Guillaume Bort

    Guillaume Bort April 27th, 2011 @ 03:59 PM

    It is better to keep your fix and apply it yourself on Play. Applying patch like this one make the overall framework more fragile, and anyway we can't guarantee that these kind patch will work as expected on future versions.

  • Morten Kjetland

    Morten Kjetland April 27th, 2011 @ 04:28 PM

    one possible solution would be to include a new param called rawQueryString, which contains the queryString as received without performing any decoding on it..

    and if the decoding fails, we only skip adding the decoded params.. I don't think I like this solution.

    What if:

    We include another method in PlayPlugin:

    public String preUrlDecoding(String url);
    

    I'm no sure about that one either...

    But on the other hand we should make it possible to re-implement legacy applications in Play.

    What do you think Guillaume?

  • Guillaume Bort

    Guillaume Bort April 28th, 2011 @ 03:12 PM

    The submitted patch did not change the outcome of the request decoding. It simply made the query string parameters not to be included > when decoding the request which was the original case.

    Is there a patch for this? I don't find it.

    One way would be to ignore errors and continue. Anyway the request.queryString value already contains the raw queryString, right?

  • mateas

    mateas April 29th, 2011 @ 07:42 AM

    Yes, I made a pull request with my patch.
    See https://github.com/playframework/play/pull/150

    This patch just ignores the query string when decoding, the same thing the original code does but with minor modifications.

  • Morten Kjetland

    Morten Kjetland June 5th, 2011 @ 07:10 AM

    • State changed from “invalid” to “new”

    I tried pull on master but it didn't work due to changes. I've updated pull with comment

  • Morten Kjetland

    Morten Kjetland June 16th, 2011 @ 08:35 PM

    • State changed from “new” to “invalid”

    Closing this due to missing response from mateas

  • MarcelLukas

    MarcelLukas January 16th, 2018 @ 01:59 PM

    Learning about UrlDecoder for request path fails when parsing request is just so amazing, I have been thinking about getting to a few essay writing service reviews as they have helped me a lot for learning something new, Keep sharing more information with us

  • BlakeOrlando

    BlakeOrlando January 29th, 2018 @ 10:48 AM

    The tutorial video has really got some excellent points to help me, I might be visiting original site
    to see how I can use the Style Guide for myself, Thanks again for the share

  • John Smith

    John Smith March 23rd, 2018 @ 11:25 AM

    We had a similar issue in our angular application where we were encoding % sign once in client side code. When we received the value in servlet it was already decoded due to request.getParameter(). Since we already had URL decoder in our sever side code, decoding the % sign twice was causing a "URLDecoder: Incomplete trailing escape (%) pattern" exception. We figured out that we we should not encode and decode % as a value at all to get face this issue.

    aba therapy services

  • derryfin
  • derryfin

    derryfin April 16th, 2018 @ 10:40 AM

    This is often additionally an excellent post we truly loved taking a look at. It is never every single day we retain the likelihood to see anything. Limo service

  • derryfin

    derryfin April 16th, 2018 @ 01:13 PM

    I am just looking towards versions write-up. It happens to be best for learn men and women explain in words for the heart besides being familiar with in this particular important design is often just found out. wall and floor tiles online

  • janicegomez
  • janicegomez
  • janicegomez

    janicegomez April 25th, 2018 @ 08:25 AM

    So i'm addicted to varieties write-up. It will be beneficial to understand many people explain in words in the heart and also recognizing with this vital idea is commonly plainly determined. tractor belt pulley

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2>Play framework</h2>

Play makes it easier to build Web applications with Java. It is a clean alternative to bloated Enterprise Java stacks. It focuses on developer productivity and targets RESTful architectures. Learn more on the <a href="http://www.playframework.org">http://www.playframework.org</a> website.<br><br>

<h2>Source code is hosted on github</h2>Check out our repository at <a href="http://github.com/playframework/play">http://github.com/playframework/play</a><br><br>

<h2>Contributing, creating a patch</h2> Please read the <a href="http://play.lighthouseapp.com/projects/57987/contributor-guide">contributor guide</a><br><br>

<h2>Reporting Security Vulnerabilities</h2> Since all bug reports are public, please report any security vulnerability directly to <em>guillaume dot bort at gmail dot com</em>.<br><br>

<h2>Creating a bug report</h2> Bug reports are incredibly helpful, so take time to report bugs and request features in our ticket tracker. We’re always grateful for patches to Play’s code. Indeed, bug reports with attached patches will get fixed far quickly than those without any.<br><br>

Please include as much relevant information as possible including the exact framework version you're using and a code snippet that reproduces the problem.<br><br>

Don't have too much expectations. Unless the bug is really a serious "everything is broken" thing, you're creating a ticket to start a discussion. Having a patch (or a branch on Github we can pull from) is better, but then again we'll only pull high quality branches that make sense to be in the core of Play.

Pages