sporkmonger / addressable

Addressable is an alternative implementation to the URI implementation that is part of Ruby's standard library. It is flexible, offers heuristic parsing, and additionally provides extensive support for IRIs and URI templates.
Apache License 2.0
1.56k stars 266 forks source link

Addressable::Template#expand transforms some multi-byte string to another unintended string. #258

Closed civitaspo closed 7 years ago

civitaspo commented 7 years ago

Hi maintainers,

I use addressable in google-api-client-ruby. I found the below bug, so could you fix it?

irb(main):001:0> require 'addressable'
=> true
irb(main):002:0> url = Addressable::Template.new('https://sheets.googleapis.com/v4/spreadsheets/{spreadsheetId}/values/{range}')
=> #<Addressable::Template:0x7d0 PATTERN:https://sheets.googleapis.com/v4/spreadsheets/{spreadsheetId}/values/{range}>
irb(main):003:0> url.expand(spreadsheetId: 'test', range: "日本語()")
=> #<Addressable::URI:0x7d2 URI:https://sheets.googleapis.com/v4/spreadsheets/test/values/%E6%97%A5%E6%9C%AC%E8%AA%9E%28%29>
irb(main):004:0> URI.unescape('https://sheets.googleapis.com/v4/spreadsheets/test/values/%E6%97%A5%E6%9C%AC%E8%AA%9E%28%29')
=> "https://sheets.googleapis.com/v4/spreadsheets/test/values/日本語()"
## why is this not 'https://sheets.googleapis.com/v4/spreadsheets/test/values/%e6%97%a5%e6%9c%ac%e8%aa%9e%ef%bc%88%ef%bc%89' ?

Addressable::Template#expand transforms multi-byte brackets () to single-byte brackets ().

civitaspo commented 7 years ago

Oh, I forgot writing my environment.

addressable version: 2.5.0 ruby version: jruby 9.1.8.0 (2.3.1) 2017-03-06 90fc7ab Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]

civitaspo commented 7 years ago

This code https://github.com/sporkmonger/addressable/blob/4de11a4b95580607f66e7e1829e5aaeacb07a086/lib/addressable/idna/pure.rb#L114-L121 intentionally transforms multi-byte string to single-byte string if possible. Why does Addressable::Template require this?

sporkmonger commented 7 years ago

As mentioned in #259, it's actually required by the URI Template spec. We're going to make it optional though.

sporkmonger commented 7 years ago

2.5.1 released w/ optional normalization.

civitaspo commented 7 years ago

thx so much!