net/url: PathEscape and PathUnescape functions

[nigeltao]

Should we add PathEscape and PathUnescape functions, similar to QueryEscape and QueryUnescape? The functionality is already present (see func (*URL) EscapedPath), but it is awkward to use.

See https://groups.google.com/forum/#!topic/golang-dev/UDUqvuKrq14 for golang-dev discussion

[rsc]

I mailed with r.w.johnstone off list about this, and for his application he only needs to escape path elements to use when building URLs. For that, QueryEscape works fine, and is what other existing code uses as far as I know. Maybe instead of new API we should document that fact.

[jmhodges]

There are a few places where QueryEscape doesn't quite work. Notably, spaces are to be encoded as "%20", instead of "+", when used in paths.

[bradfitz]

@jmhodges, fair enough. Want to send a change?

[jmhodges]

Yeah, was trying to pawn it off on a friend but he didn't bite.

[jmhodges]

So, I'm digging in here and trying to figure out how far to take this. I've come up with 4 options I could implement but none of them are super satisfying.

We have the concept of encoding in the code base. The obvious but non-working change would be to reuse the current escape func for PathSegmentEscape with the encodePath encoding. Doing so, however, would cause all of the / in the user's path segment to remain as / instead of being turned into %2F. This is because the shouldEscape func called in escape knows about encodePath and expects a full path to be given to it. We also can't just pass the encodeQueryComponent mode to escape because that's been special cased to return + when it sees a space.

The first option is to create a special mode called encodePathSegment to be used by escape and shouldEscape and only use it from PathSegmentEscape. That new code would never be seen by the other path escaping routines, however. This means we might introduce some inaccuracies about how we treat full paths in EscapedPath and validEncodedPath versus path segments with PathSegmentPath.

A second option is to teach the places calling escape(..., encodePath) and shouldEscape(..., encodePath) directly that they need to skip over / themselves and make encodePath really be about path segments and not the full path. We'd also, of course, have to teach shouldEscape(..., encodePath) that / should be encoded and the places calling shouldEscape would, too. The places that need to be taught are validEncodedPath, and URL.EscapedPath.

A third option is for escape itself to do the / skipping in encodePath mode while shouldEscape learns to always escape / when it sees it with encodePath. This would involve escape checking for '/' in both of its for-loops before calling shouldEscape(..., encodePath) and the other places that call shouldEscape(..., encodePath) (just validEncodedPath, currently) would have to do the same. It would probably be wise, in this case, to turn the encodePath references in shouldEscape to encodePathSegment and have escape only ever call shouldEscape with encodePathSegment instead of encodePath.

The last option I came up with is for PathSegmentEscape to call shouldEscape directly and not use escape at all. This would let us special case / but at the cost of duplicating the escape function's logic for quick returns and hex string length.

I'm pretty torn on these 4 options. Does anyone have a preference or alternative idea?

[jmhodges]

I'm leaning toward the third, but having a encoding that was just for escape and unescape, but not allowed in shouldEncode seemed like maybe a place for misuse.

[bradfitz]

I'm also fine with you creating an entirely new set of functions not reusing anything that's currently existing. That might even be safest for now. Write sufficient tests and we can refactor later.

I have a plan to create a new shared package golang.org/x/net/lex for all lexical matters & tables of the various RFCs about URLs and HTTP so http2 and http1 and net/url can share them, so I can do the deduplication later when lex exists.

[jmhodges]

Yeah, I'm on it. Option 3 for now because I'm not feeling sassy enough.

Oh man, a net/lex would be handy.