Resolving issue #803 by adding quotes around the parameters

[Spechal]

Fixes #803

[Spechal]

Closing to force rebuild -- Travis CI errored

[jfryman]

Well, there is a dependency on Ruby 2.x now with this google-api-client, which is causing the tests to fail. Need to take a look at this...

[3flex]

If you try a rebuild now it should work.

beaker depends on google-api-client, which as of 0.9.5 requires Ruby 2. But beaker released a new version a couple of days ago which forces google-api-client 0.9.4 and lower, which doesn't have a specific Ruby requirement.

The gem dependencies should be updated in Travis so they don't rely on beaker, since the acceptance tests aren't run there.

[3flex]

Hmm, a rebuild didn't work, it's still loading the problematic gem... Merging this anyway since the problem is definitely with the CI config.

Hopefully the merge triggers a fresh build in master and the problem goes away, but if not I'll create a PR and try and fix up the Gemfile.

[Spechal]

Thanks for the merge. I received another message asking for justification as the nginx docs don't mention needing quotes.

Without this merge, the following add_header will fail:

add_header Access-Control-Allow-Methods GET, POST, OPTIONS;

It results in nginx: [emerg] invalid number of arguments in "add_header" directive in...onf:6

Adding quotes: add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';

The problem is resolved.

[bryangwilliam]

I am seeing issues with this change when using add_header to insert a Content-Security-Policy header. The CSP syntax uses single-quotes, which clash with the single-quotes surrounding the header value in the template. Is there any reason for using the single-quotes as opposed to double-quotes?

A value of { "Content-Security-Policy" : "default-src 'self';" } passed into the $add_header parameter of nginx::resource::server will result in the configuration add_header 'Content-Security-Policy' 'default-src 'self';', which breaks the nginx syntax.

I tried using double-quotes in the CSP header as a workaround, but it breaks on certain keywords in the browser.

[wyardley]

@bryangwilliam can you open a new issue about this? Not sure how soon someone will have time to look at it, but at least we won't lose track of it.

You might just use the raw append / prepend options for now, but agree that we should come up with a better fix. Are there cases where having double quotes around the header value itself causes a problem?

Presumably, we should add some spec test cases for various scenarios, and then rework it until they all pass.

[bryangwilliam]

I did find the raw_prepend option which fixed it for me in the short term, but I'd definitely be interested in a more elegant solution. I wasn't sure if this should go in as a new issue, so I'll go ahead and do that now.

[wyardley]

@bryangwilliam Awesome! If you're motivated to play around with it some more, happy to help you get setup in terms of writing some tests so that you can play around with submitting a PR.