Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

MetadataQueryProtocolRequestURLBuilder encodes entityIDs improperly

Description

Ian has been clarifying the MDQ spec to note that since the entityID is encoded into the path of the URL, you have to take care not to improperly encode the space character into a '+', because that character is legal in a path, and so a decoder that's done correctly won't decode it back into a space.

In practice this isn't a big deal, we're not going to see spaces in entityIDs, but it's something we should fix. I didn't realize we were still using Java's URL encoder, but this pretty much makes it untrustworthy, or at least it means it's only designed to encode query parameters, not path content.

Environment

None

Activity

Show:

Ian YoungSeptember 29, 2016 at 10:46 AM

Yes, ':' is legal in a path, as is '{' and '}' which also come up in the protocol. So they don't need to be encoded, although as Scott says it will all work out just fine if they are.

Scott CantorSeptember 28, 2016 at 12:42 PM

Generally, encoding legal characters is fine, it's when they're encoded improperly or not encoded that we have a problem. The decoders will always decode a %-encoded sequence back for us.

Brent PutmanSeptember 28, 2016 at 12:18 AM

As we had decided, I'm plugging in Guava's UrlEscapers.urlPathSegmentEscaper() here. In the unit tests I just wanted to note that the encoding isn't exactly the same:

java.lang.AssertionError: expected http://metadata.example.org/service/entities/http%3A%2F%2Fexample.org%2Fidp but found http://metadata.example.org/service/entities/http:%2F%2Fexample.org%2Fidp

that is, it doesn't encode the colon character. I'm going to assume that that is OK, since AFAIK colon is valid in the path and doesn't need to be encoded. I just wanted to note for the record in case someone thinks it's not, or if there is indeed some weirdness vis-a-vis the MDQ spec. In that case, we can reopen.

Fixed

Details

Assignee

Reporter

Components

Fix versions

Affects versions

Created August 31, 2016 at 5:03 PM
Updated September 29, 2016 at 10:46 AM
Resolved September 28, 2016 at 12:20 AM