Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Including = symbol in JSON post body causes body to become truncated #77

Closed
andypiper opened this issue Nov 1, 2017 · 5 comments
Closed
Assignees
Labels

Comments

@andypiper
Copy link
Contributor

Currently, including the = symbol inside any JSON body posted to the API via twurl causes the body content to be truncated at the = symbol, and the request will subsequently fail. This occurs even with the Content-Type set to application/json.

@andypiper andypiper added the Bug label Nov 1, 2017
@andypiper andypiper self-assigned this Nov 1, 2017
@andypiper
Copy link
Contributor Author

I'm seeing that this works if the = is encoded as %3d.

@smaeda-ks
Copy link
Collaborator

smaeda-ks commented Dec 18, 2018

I caught this issue today as well. For those who are curious, the problem is coming from this logic:

key, value = pair.split('=', 2)

It looks like twurl is doing this by default and there's no way to skip this which is very confusing and doesn't make sense to me having this logic for all POST data.

This occurs even with the Content-Type set to application/json.

Exactly... curl doesn't do anything like this.
https://ec.haxx.se/http-post.html

Probably, it's difficult to just remove this logic as it will potentially require a bunch of documentation fix (on developer.twitter.com) and other critical stuff, but we should have a logic to check if a Content-Type is set by option (-A) and if it contains application/json in its value, so that we could skip this unnecessary parse for json POST body at least.

I also noticed there's a -r, --raw-data [data] Sends the specified data as it is in a POST request to the HTTP server. option:

twurl/lib/twurl/cli.rb

Lines 240 to 246 in 7eb9a13

def raw_data
on('-r', '--raw-data [data]', 'Sends the specified data as it is in a POST request to the HTTP server.') do |data|
CGI::parse(data).each_pair do |key, value|
options.data[key] = value.first
end
end
end

but as you can guess it doesn't work because of the parse, again...

Well, there's a logic that supposed to pass a "raw" body data (was introduced in #60):

elsif request.content_type && options.data
request.body = options.data.keys.first

but it only gets a "key" even though rest of the text after = parsed by above logic is in its value.

@smaeda-ks
Copy link
Collaborator

FYI, I looked at the code further (I'm Ruby beginner though..) and noticed that this fix isn't that easy as I thought initially. Since twurl is using OptionParser in order to parse command-line args, it reads args one by one with an actual input order. That means, if a user set options like this oder:

twurl -d '{"data": "foo=bar"}' -A 'Content-Type: application/json' '/path/to/endpoint'

data function doesn't know if there's a Content-Type header in options.headers or not at this stage.

twurl/lib/twurl/cli.rb

Lines 231 to 238 in 7eb9a13

def data
on('-d', '--data [data]', 'Sends the specified data in a POST request to the HTTP server.') do |data|
data.split('&').each do |pair|
key, value = pair.split('=', 2)
options.data[key] = value
end
end
end

Maybe we can do pattern matching using ARGV or something, but not sure if it's a reasonable way. So the easiet wasy I tested is to modify this block:

elsif request.content_type && options.data
request.body = options.data.keys.first

to something like this:

elsif request.content_type && options.data
  data = ''

  if options.data.length == 1 && options.data.values.first == nil
    data += options.data.keys.first
  else
    options.data.each_with_index do |(key, value), i|
      data += (i == 0 ? '' : '&') + key.to_s + (value == nil ? '' : '=' + value.to_s)
    end
  end

  request.body = data

@smaeda-ks
Copy link
Collaborator

The fix is merged into master.

@smaeda-ks
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants