Uploaded image for project: 'Confluence Data Center'
  1. Confluence Data Center
  2. CONFSERVER-16282

GeneralUtil::urlDecode should not return the undecoded url on exception

XMLWordPrintable

      GeneralUtil.java:423 should not return the original undecoded string if the string cannot be decoded. Patch attached

        1. GeneralUtil.patch
          1.0 kB
          David Cheney

            [CONFSERVER-16282] GeneralUtil::urlDecode should not return the undecoded url on exception

            Steve Haffenden (Inactive) added a comment -

            Hi There

            Thanks for taking the time to raise this issue. This has been on out backlog now for some time with very little progress being made. Rather than leave this here I'm going to close this issue as won't fix, I believe that this better reflects the status of this issue.

            If the problem is raised again in the future we'll re-address it.

            Regards
            Steve Haffenden
            Confluence Bugmaster
            Atlassian

            Steve Haffenden (Inactive) added a comment - Hi There Thanks for taking the time to raise this issue. This has been on out backlog now for some time with very little progress being made. Rather than leave this here I'm going to close this issue as won't fix, I believe that this better reflects the status of this issue. If the problem is raised again in the future we'll re-address it. Regards Steve Haffenden Confluence Bugmaster Atlassian

            Chii (Inactive) added a comment -

            I've reverted the commit here , so that the stable plugins build can pass.

            Chii (Inactive) added a comment - I've reverted the commit here , so that the stable plugins build can pass.

            Chii (Inactive) added a comment -

            The patch applied at this commit causes the regression that a page with a '%' in the name will only be reachable via its page id link, not via the direct /display/SPACE/L%schen link.

            See this test for the breakage

            Chii (Inactive) added a comment - The patch applied at this commit causes the regression that a page with a '%' in the name will only be reachable via its page id link, not via the direct /display/SPACE/L%schen link. See this test for the breakage

            Matt Ryall added a comment -

            When is not decoding a URL a problem? Can someone explain the ramifications of this?

            Why are we catching Exception? I think that is the more serious problem with this bit of code.

            Matt Ryall added a comment - When is not decoding a URL a problem? Can someone explain the ramifications of this? Why are we catching Exception? I think that is the more serious problem with this bit of code.

            Anatoli added a comment -

            of course I meant decode.

            Anatoli added a comment - of course I meant decode .

            David Cheney (Inactive) added a comment -

            In your example, in the case of decoding failure, you're proposing to double encode ?

            David Cheney (Inactive) added a comment - In your example, in the case of decoding failure, you're proposing to double encode ?

            Anatoli added a comment - - edited

            David, to gauge the importance of the problem can you please mention situations where current implementation presented a problem?

            I also think that in case of unsuppoted encoding exception we might want to use the default encoding 

            catch (UnsupportedEncodingException e)
{
  log.error("Error while ...");
  return return URLEncoder.decode(url, getDefaultEncoding());
}

            EDITED - changed from encode to decode

            Anatoli added a comment - - edited David, to gauge the importance of the problem can you please mention situations where current implementation presented a problem? I also think that in case of unsuppoted encoding exception we might want to use the default encoding catch (UnsupportedEncodingException e) { log.error( "Error while ..." ); return return URLEncoder.decode(url, getDefaultEncoding()); } EDITED - changed from encode to decode

              Unassigned Unassigned
              dcheney David Cheney (Inactive)
              Affected customers:
              1 This affects my team
              Watchers:
              5 Start watching this issue

                Created:
                Updated:
                Resolved: