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

Can't pass 'always' parameter to add_header due to single quoting #1020

Closed
benh57 opened this issue Feb 15, 2017 · 5 comments
Closed

Can't pass 'always' parameter to add_header due to single quoting #1020

benh57 opened this issue Feb 15, 2017 · 5 comments
Labels
bug Something isn't working

Comments

@benh57
Copy link

benh57 commented Feb 15, 2017

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 4.9.2
  • Distribution: Centos 7
  • Module version: 0.6.1

How to reproduce (e.g Puppet code you use)

nginx::resource::server { 'foo':
  add_header           => { 'Access-Control-Allow-Origin'      => '"*" always' } ,

}

What are you seeing

Results in config:

add_header 'Access-Control-Allow-Origin' '"*" always';

Which breaks in Chrome:
The 'Access-Control-Allow-Origin' header contains multiple values '"*" always', but only one is allowed.

What behaviour did you expect instead

This formerly generated:

add_header 'Access-Control-Allow-Origin' "*" always;

Any additional information you'd like to impart

Related to #991

@wyardley
Copy link
Collaborator

If you're able, can you test #992 ?

@benh57
Copy link
Author

benh57 commented Feb 16, 2017

Indeed, this does appear to work! I expected it would not. I just have to modify my puppet-side code to not use the double quotes:
add_header => { 'Access-Control-Allow-Origin' => '* always' } ,

Then i get this in the config:

add_header 'Access-Control-Allow-Origin' "* always";

And Chrome appears to get the star, properly, and not the 'always'.

So it's still a breaking change from my previous config where i had my own double quotes around the star.

thanks!

@benh57
Copy link
Author

benh57 commented Feb 16, 2017

I suppose this parameter should be validated with a regex to ensure that the user isn't passing a nonescaped quote/ double quote. Since that breaks nginx when it's quoted again.

@wyardley
Copy link
Collaborator

@benh57 Yeah, though see the comments from @xaque208 in that issue, my reading is that maybe double quotes aren't legal in header values anyway? I'm a bit worried that adding such validation might almost be a cure that's worse than the disease?

I'm hoping someone will merge that issue soon, since it seems like at least an improvement over the current situation.

@zachfi
Copy link
Contributor

zachfi commented Feb 26, 2017

I believe that @wyardley has fixed this one. True @benh57?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants