bblimke / webmock

Library for stubbing and setting expectations on HTTP requests in Ruby.
MIT License
3.97k stars 556 forks source link

Failed to mock cUrl requests with query params #748

Closed omerlh closed 6 years ago

omerlh commented 6 years ago

I'm trying to mock the following request (the full source code is here):

     supported=JSON.parse(Curl.get("#{base}/JSON/core/view/version/?apikey=#{apikey}").body_str)

It worked perfectly fine when I'm ignoring the query param using:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(body: "{\"version\":\"2.7.0\"}", status: 200,
            headers: { 'Content-Length' => 19 })

But anything I've tried (either with the query: parameter, or using Addressable::Template.new "http://zap:1234/JSON/core/view/version/?apikey=key") to check the query param failed with the following error:

Real HTTP connections are disabled. Unregistered request: GET http://zap:1234/JSON/core/view/version/

You can stub this request with the following snippet:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(status: 200, body: "", headers: {})

registered request stubs:

stub_request(:get, ""http://zap:1234/JSON/core/view/version/?apikey=key" with variables []")

I'm using the latest Gem version, on Mac. Currently I'm trying to debug the issue, but help is appreciated.

bblimke commented 6 years ago

@omerlh It's weird that WebMock doesn't see the apikey query parameter. Can you conform that the apikey parameter is passed to the request?

Can you please provide a sample code or spec to reproduce the issue?

omerlh commented 6 years ago

This is the output when curl running with verbose mode:

*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 1234 (#0)
> GET /JSON/core/view/version/?apikey= HTTP/1.1
Host: localhost:1234
Accept: */*

< HTTP/1.1 200 OK
< Pragma: no-cache
< Cache-Control: no-cache
< Content-Security-Policy: default-src 'none'; script-src 'self'; connect-src 'self'; child-src 'self'; img-src 'self' data:; font-src 'self' data:; style-src 'self'
< Referrer-Policy: no-referrer
< Access-Control-Allow-Methods: GET,POST,OPTIONS
< Access-Control-Allow-Headers: ZAP-Header
< X-Frame-Options: DENY
< X-XSS-Protection: 1; mode=block
< X-Content-Type-Options: nosniff
< X-Clacks-Overhead: GNU Terry Pratchett
< Content-Length: 19
< Content-Type: application/json; charset=UTF-8
<
* Connection #0 to host localhost left intact

So looks like the query param is passing, this time empty. To reproduce, you can use this branch, by running:

rspec spec/tasks/zap/zap_spec.rb

After running bundle install.

bblimke commented 6 years ago

On which line of zap_spec.rb does it fail and what is the error message?

omerlh commented 6 years ago

This is the full test output:

WARN: Unresolved specs during Gem::Specification.reset:
      cucumber (>= 1.3.19)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
...Real HTTP connections are disabled. Unregistered request: GET http://zap:1234/JSON/core/view/version/

You can stub this request with the following snippet:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(status: 200, body: "", headers: {})

registered request stubs:

stub_request(:get, ""http://zap:1234/JSON/core/view/version/?apikey=key" with variables []")

============================================================. Tried to connect to http://zap:1234/JSON/core/view/version/. Check that ZAP is running on the right host and port and that you have the appropriate API key, if required.
.Real HTTP connections are disabled. Unregistered request: GET http://zap:1234/JSON/core/view/version/

You can stub this request with the following snippet:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(status: 200, body: "", headers: {})

registered request stubs:

stub_request(:get, ""http://zap:1234/JSON/core/view/version/?apikey=key" with variables []")

============================================================. Tried to connect to http://zap:1234/JSON/core/view/version/. Check that ZAP is running on the right host and port and that you have the appropriate API key, if required.
F....

Failures:

  1) Glue::Zap#supported? when Zap version is not supported issues a notification
     Failure/Error: expect(Glue).to receive(:notify)

       (Glue).notify(*(any args))
           expected: 1 time with any arguments
           received: 0 times with any arguments
     # ./spec/tasks/zap/zap_spec.rb:93:in `block (4 levels) in <top (required)>'

Finished in 0.57969 seconds (files took 2.58 seconds to load)
9 examples, 1 failure

Failed examples:

rspec ./spec/tasks/zap/zap_spec.rb:91 # Glue::Zap#supported? when Zap version is not supported issues a notification

You can see that all network requests failed because Webmock has not configured to mock them. So all the tests are miss-behave, the specific failing test is ./spec/tasks/zap/zap_spec.rb:93.

bblimke commented 6 years ago

Perhaps this template doesn't match? Is that template valid? Is there a reason to use the Addressable::Template there and not a normal string?

omerlh commented 6 years ago

I've tried with irb and I think the template does match:

irb(main):001:0> require 'addressable/template'
=> true
irb(main):002:0> uriTemplate = Addressable::Template.new "http://zap:1234/JSON/core/view/version/?apikey=key"
=> #<Addressable::Template:0x3ff79e8d7414 PATTERN:http://zap:1234/JSON/core/view/version/?apikey=key>
irb(main):003:0> uri = "http://zap:1234/JSON/core/view/version/?apikey=key"
=> "http://zap:1234/JSON/core/view/version/?apikey=key"
irb(main):004:0> uriTemplate.match(uri)
=> #<Addressable::Template::MatchData:0x3ff79e8cb22c RESULT:{}>
irb(main):005:0>

It showed also the value of the query param if I specific it.

The use of the Addressable::Template is a desperate move, after using the regular query: did not work. I thought that might be the issue, so I tried also with Addressable::Template

bblimke commented 6 years ago

Have you tried stub_request(:get, "http://zap:1234/JSON/core/view/version/?apikey=key") ?

omerlh commented 6 years ago

Yep, same issue:

WARN: Unresolved specs during Gem::Specification.reset:
      cucumber (>= 1.3.19)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
...Real HTTP connections are disabled. Unregistered request: GET http://zap:1234/JSON/core/view/version/

You can stub this request with the following snippet:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(status: 200, body: "", headers: {})

registered request stubs:

stub_request(:get, "http://zap:1234/JSON/core/view/version/?apikey=key")

============================================================. Tried to connect to http://zap:1234/JSON/core/view/version/. Check that ZAP is running on the right host and port and that you have the appropriate API key, if required.
.Real HTTP connections are disabled. Unregistered request: GET http://zap:1234/JSON/core/view/version/

You can stub this request with the following snippet:

stub_request(:get, "http://zap:1234/JSON/core/view/version/").
  to_return(status: 200, body: "", headers: {})

registered request stubs:

stub_request(:get, "http://zap:1234/JSON/core/view/version/?apikey=key")

============================================================. Tried to connect to http://zap:1234/JSON/core/view/version/. Check that ZAP is running on the right host and port and that you have the appropriate API key, if required.
F....

Failures:

  1) Glue::Zap#supported? when Zap version is not supported issues a notification
     Failure/Error: expect(Glue).to receive(:notify)

       (Glue).notify(*(any args))
           expected: 1 time with any arguments
           received: 0 times with any arguments
     # ./spec/tasks/zap/zap_spec.rb:93:in `block (4 levels) in <top (required)>'

Finished in 0.58103 seconds (files took 4.72 seconds to load)
9 examples, 1 failure

Failed examples:

rspec ./spec/tasks/zap/zap_spec.rb:91 # Glue::Zap#supported? when Zap version is not supported issues a notification
oliakremmyda commented 6 years ago

I think the issue is that in case of curb_adaptor, the query params are not stored in the request signature resulting the match to fail. For example, in case of Curl request

Curl.get('www.example.com?a=b')

the request_signature.uri is

=> #<Addressable::URI:0x3ffd8a9b1528 URI:http://www.example.com:80/>

While, in case of a Net::HTTP request


Net::HTTP.get('www.example.com','?a=b')

the request_signature.uri is

=> #<Addressable::URI:0x3fd54dfea378 URI:http://www.example.com:80/?a=b>
omerlh commented 6 years ago

I was suspecting that while reading the code, see this line:

request_signature = WebMock::RequestSignature.new(
          method,
          uri.to_s,
          body: request_body,
          headers: headers
        )
        request_signature

The issue might be that the URI does not contains the query params, although when I've checked it locally it looks like it's there...

omerlh commented 6 years ago

The bug was in my curl usage, webmock correctly reported that the query param is not passing. It is unclear to me why curl debug show the query param. The bug was that Curl.get() (see the docs here) receive another parameter for query param. If this parameter is empty, curl will erase the query params. This is the correct usage:

Curl.get("#{base}/JSON/core/view/version/", "apikey" => apikey