icicleio / http

HTTP component for Icicle
MIT License
61 stars 5 forks source link

[Fixes #12] Fixed `+` encoding to `%2B` #13

Closed korotovsky closed 8 years ago

korotovsky commented 8 years ago

@trowski Probably we have to exclude [ and ] from this regexp as well

trowski commented 8 years ago

Yes, in fact I think !$&'()[]*+,:;=/% all need to be removed from that regex. Does this sound correct?

korotovsky commented 8 years ago

No, we have to remove [, ], +, =, /, %, & and '.

nexor commented 8 years ago

Сould we just use rawurlencode function instead of this method ?

trowski commented 8 years ago

@nexor The function is using rawurlencode() to replace characters, but some characters should not be encoded, particularly those that may already be an encoded character.

@korotovsky Why only those characters? Based on the URI RFC, the only excluded characters should be alphanumeric plus -, _, ., !, ~, *, ', (, and ) (see section 2.3).

korotovsky commented 8 years ago

@trowski Probably we should follow URI RFC, but my suggestion was based on Google Chrome 47 behavior.

korotovsky commented 8 years ago

As I see this commit is already in master, so I close this PR.

trowski commented 8 years ago

I excluded other characters from the URI RFC as well except [ and ]. Should that be fine or is there a reason to encode them as well?

korotovsky commented 8 years ago

Google Chrome encodes it :) RFC says that we should not... I'm confused.

trowski commented 8 years ago

Actually the RFC says [ and ] should be encoded. I thought Google Chrome was not encoding them. Probably means it should be encoded here as well.

korotovsky commented 8 years ago

Then we have to encode them. I misunderstood you.