OPTIONS request during preflight check returns 500 and causes spec failure #195

alexeits · 2018-06-29T18:22:10Z

Software versions

OS: macOS 10.13.5
Consumer Pact library: pact-node 6.18.3, pact-web 5.9.1
Node Version: v8.10.0

Expected behaviour

Define a DELETE interaction without a body:

pact
  .addInteraction({
    uponReceiving: 'delete valid token',
    withRequest: {
      method: 'DELETE',
      path: '/foo',
    },
    willRespondWith: { status: 204 },
  })

Invoke the web consumer in the browser, which sends DELETE http://localhost:1234/foo via XHR

Expect the spec to pass

Actual behaviour

The spec fails during preflight check because OPTIONS returns 500

It appears that pact-mock_service doesn't like "Access-Control-Allow-Headers":null (see the log snippet below)

Steps to reproduce via cURL:

With Access-Control-Allow-Headers it succeeds

$ curl -XOPTIONS localhost:1234/foo -H 'Access-Control-Request-Method: DELETE' -H 'Origin: http://localhost:8000' -H 'Access-Control-Request-Headers: content-type' -I
HTTP/1.1 200 OK 
Access-Control-Allow-Origin: http://localhost:8000
Access-Control-Allow-Headers: content-type
Access-Control-Allow-Methods: DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH
Server: WEBrick/1.3.1 (Ruby/2.2.2/2015-04-13) OpenSSL/1.0.2d
Date: Fri, 29 Jun 2018 18:13:34 GMT
Content-Length: 0
Connection: Keep-Alive

Without Access-Control-Allow-Headers it returns 500

$ curl -XOPTIONS localhost:1234/foo -H 'Access-Control-Request-Method: DELETE' -H 'Origin: http://localhost:8000'
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">
<HTML>
  <HEAD><TITLE>Internal Server Error</TITLE></HEAD>
  <BODY>
    <H1>Internal Server Error</H1>
    undefined method `split' for nil:NilClass
    <HR>
    <ADDRESS>
     WEBrick/1.3.1 (Ruby/2.2.2/2015-04-13) OpenSSL/1.0.2d at
     localhost:1234
    </ADDRESS>
  </BODY>
</HTML>

Relevant log files

I, [2018-06-29T11:06:46.818894 #67786]  INFO -- : Registered expected interaction DELETE /api/v3/desktop_tokens/michelle-smith-id
D, [2018-06-29T11:06:46.819417 #67786] DEBUG -- : {
  "description": "delete matching desktop token",
  "providerState": "desktop token userId matches userId URL param",
  "request": {
    "method": "DELETE",
    "path": "/api/v3/desktop_tokens/michelle-smith-id"
  },
  "response": {
    "status": 204,
    "headers": {
    }
  }
}
I, [2018-06-29T11:06:46.829789 #67786]  INFO -- : Received OPTIONS request for mock service administration endpoint DELETE /api/v3/desktop_tokens/michelle-smith-id. Returning CORS headers: {"Access-Control-Allow-Origin":"http://localhost:8000","Access-Control-Allow-Headers":null,"Access-Control-Allow-Methods":"DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH"}.
I, [2018-06-29T11:06:46.842698 #67786]  INFO -- : Received OPTIONS request for mock service administration endpoint GET /interactions/verification. Returning CORS headers: {"Access-Control-Allow-Origin":"http://localhost:8000","Access-Control-Allow-Headers":"content-type,x-pact-mock-service","Access-Control-Allow-Methods":"DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH"}.
W, [2018-06-29T11:06:46.852675 #67786]  WARN -- : Verifying - actual interactions do not match expected interactions. 
Missing requests:
	DELETE /api/v3/desktop_tokens/michelle-smith-id


W, [2018-06-29T11:06:46.852739 #67786]  WARN -- : Missing requests:
	DELETE /api/v3/desktop_tokens/michelle-smith-id


I, [2018-06-29T11:06:46.865028 #67786]  INFO -- : Received OPTIONS request for mock service administration endpoint POST /pact. Returning CORS headers: {"Access-Control-Allow-Origin":"http://localhost:8000","Access-Control-Allow-Headers":"content-type,x-pact-mock-service","Access-Control-Allow-Methods":"DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH"}.
I, [2018-06-29T11:06:46.882042 #67786]  INFO -- : Cleared interactions
I, [2018-06-29T11:06:46.892314 #67786]  INFO -- : Received OPTIONS request for mock service administration endpoint DELETE /session. Returning CORS headers: {"Access-Control-Allow-Origin":"http://localhost:8000","Access-Control-Allow-Headers":"content-type,x-pact-mock-service","Access-Control-Allow-Methods":"DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH"}.
I, [2018-06-29T11:06:46.899463 #67786]  INFO -- : Cleared session

The text was updated successfully, but these errors were encountered:

mefellows · 2018-06-30T00:25:19Z

Thanks @alexeits. Could you please provide the full call to the Pact constructor? Are you setting the cors option to true?

https://github.com/pact-foundation/pact-js#constructor-options

alexeits · 2018-06-30T00:28:20Z

@mefellows: yes, I am:

  let server = pact.createServer({ cors: true, dir: pactDir, log: 'pact.log', port: 1234 });
  return server.start();

It works fine as long as the request has a body. As soon as I send a request without a body Chrome starts to get 500 in OPTIONS response.

mefellows · 2018-06-30T00:32:32Z

Thanks, will have to investigate a bit further. I was able to reproduce outside of Pact JS by running the mock service directly with pact-mock-service --cors --port 1234:

matt ~/ λ pact-mock-service --cors --port 1234
[2018-06-30 10:30:57] INFO  WEBrick 1.3.1
[2018-06-30 10:30:57] INFO  ruby 2.2.2 (2015-04-13) [x86_64-darwin13]
[2018-06-30 10:30:57] INFO  WEBrick::HTTPServer#start: pid=4445 port=1234
I, [2018-06-30T10:31:26.578753 #4445]  INFO -- : Received OPTIONS request for mock service administration endpoint  /. Returning CORS headers: {"Access-Control-Allow-Origin":"*","Access-Control-Allow-Headers":null,"Access-Control-Allow-Methods":"DELETE, POST, GET, HEAD, PUT, TRACE, CONNECT, PATCH"}.
[2018-06-30 10:31:26] ERROR NoMethodError: undefined method `split' for nil:NilClass
	/opt/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.5/lib/rack/handler/webrick.rb:98:in `block in service'
	/opt/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.5/lib/rack/handler/webrick.rb:90:in `each'
	/opt/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.5/lib/rack/handler/webrick.rb:90:in `service'
	/opt/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/httpserver.rb:138:in `service'
	/opt/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/httpserver.rb:94:in `run'
	/opt/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/server.rb:191:in `block in start_thread'

cc: @bethesque any ideas?

bethesque · 2018-07-03T00:23:14Z

I've seen this before. Just trying to remember. Is the Host header present?

bethesque · 2018-07-03T00:24:06Z

"Access-Control-Allow-Headers":null that looks wrong. Let me investigate.

alexeits · 2018-07-03T00:24:33Z

Yes, the host header was present. The only header that wasn't present was Access-Control-Request-Headers. Chrome doesn't send it if there's no request body.

If there's a request body then Access-Control-Request-Headers: content-type, which works as expected

bethesque · 2018-07-03T00:26:35Z

It's trying to populate the header with the value of Access-Control-Request-Headers from the request. That isn't present.

https://github.com/pact-foundation/pact-mock_service/blob/master/lib/pact/mock_service/request_handlers/options.rb#L23

alexeits · 2018-07-03T00:29:21Z

yes, I think we are agreeing on this. It should probably skip Access-Control-Allow-Headers if Access-Control-Request-Headers is not present.

bethesque · 2018-07-03T01:10:29Z

I can make that change. I just want to be sure that it's valid to send a request without Access-Control-Request-Headers. It's been ages since I've done CORS, so I can't remember.

bethesque · 2018-07-03T01:50:24Z

I'm just reading https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request

It seems that Access-Control-Request-Headers is expected to be included, and my code (from years ago!) also seems to expect that the header is present. And yet, your browser does not seem to be sending it. So we'll have to work around it. Just trying to work out what should happen in this case. Should the Access-Control-Allow-Headers header not be present, or should it be empty. My guess is that the difference may impact how the client operates, so I want to get it right.

bethesque · 2018-07-03T04:07:05Z

Bookmark for self to read later: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

alexeits · 2018-07-03T14:05:02Z

I don't think it's required unconditionally. According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers:

This header is required if the request has an Access-Control-Request-Headers header.

In the worst case it should be acceptable to send the wildcard: Access-Control-Allow-Headers: *, which would allow any header in the actual request.

bethesque · 2018-07-04T04:05:56Z

@alexeits thanks for finding that reference. I read up on all the issues that were tracking the browser support for the wildcard, and it seems that most browsers support it now, so I've gone with the logic of returning Access-Control-Allow-Headers=* when no Access-Control-Request-Headers header is present.

@mefellows please update to 1.47.2 of the standalone

alexeits changed the title ~~OPTIONS request during prefligh check returns 500 and causes spec failure~~ OPTIONS request during preflight check returns 500 and causes spec failure Jun 29, 2018

mefellows added bug Indicates an unexpected problem or unintended behavior Triage labels Jun 30, 2018

bethesque closed this as completed in pact-foundation/pact-mock_service@855fd83 Jul 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OPTIONS request during preflight check returns 500 and causes spec failure #195

OPTIONS request during preflight check returns 500 and causes spec failure #195

alexeits commented Jun 29, 2018

mefellows commented Jun 30, 2018

alexeits commented Jun 30, 2018 •

edited

Loading

mefellows commented Jun 30, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018 •

edited

Loading

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018 •

edited

Loading

bethesque commented Jul 4, 2018

OPTIONS request during preflight check returns 500 and causes spec failure #195

OPTIONS request during preflight check returns 500 and causes spec failure #195

Comments

alexeits commented Jun 29, 2018

Software versions

Expected behaviour

Actual behaviour

Steps to reproduce via cURL:

Relevant log files

mefellows commented Jun 30, 2018

alexeits commented Jun 30, 2018 • edited Loading

mefellows commented Jun 30, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018 • edited Loading

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

bethesque commented Jul 3, 2018

alexeits commented Jul 3, 2018 • edited Loading

bethesque commented Jul 4, 2018

alexeits commented Jun 30, 2018 •

edited

Loading

alexeits commented Jul 3, 2018 •

edited

Loading

alexeits commented Jul 3, 2018 •

edited

Loading