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

enable setting LimitRequestFieldSize globally as it does not actually… #1293

Merged
merged 2 commits into from Dec 15, 2015

Conversation

KlavsKlavsen
Copy link
Contributor

… work to increase it, inside a vhost. We tested with Kerberos keys (which is why we need to increase - as they can be upto 12000 bytes long.. so a default of 8190 doesn't work :)

@igalic
Copy link
Contributor

igalic commented Dec 8, 2015

@KlavsKlavsen from httpd's documentation

When name-based virtual hosting is used, the value for this directive is taken from the default (first-listed) virtual host best matching the current IP address and port combination.

If your default (first listed) vhost doesn't have any settings, it'll use the Server's default setting, if the server hasn't changed it, it'll default to 8190.

at any rate, i agree, that this is more sensible to put in init.pp rather than vhost.pp

@KlavsKlavsen
Copy link
Contributor Author

@igalic Thank you. my parser did NOT quite catch that in the documentation ;) We just confirmed this however, by testing and found that it works when we set it in our default vhost (so we can avoid forking apache upstream module to set this - until puppetlabs-apache supports it as a general setting.

@igalic
Copy link
Contributor

igalic commented Dec 8, 2015

@KlavsKlavsen cool

would you mind extending the documentation, and adding a test?
as well, as, perhaps, removing it from the vhost, since it can be really tricky to get right

my recommendation is to use the github flow workflow, if you want to make your life / contribution easier, with follow-up requests from me and my fellow colleagues ;)

@KlavsKlavsen
Copy link
Contributor Author

I usually create a branch - when I find I need to do other PR's.. but yeah.. I should probably start with doing that, just to be sure :)

I'm running "rake spec" now - to test my updated test procedure.. damn it takes time.. :)

@igalic
Copy link
Contributor

igalic commented Dec 8, 2015

you can select only a few tests with rake rspec spec/classes/apache_spec.rb rspec spec/defines/vhost_spec.rb

@KlavsKlavsen
Copy link
Contributor Author

I've added the new test and removed the vhost option. the test succeeds:
1227 examples, 0 failures

@igalic
Copy link
Contributor

igalic commented Dec 10, 2015

thank you @KlavsKlavsen
n.b.: this will need a minor(?) version bump.
Any comments from @puppetlabs here?

igalic added a commit that referenced this pull request Dec 15, 2015
enable setting LimitRequestFieldSize globally as it does not actually…
@igalic igalic merged commit 6a0ddfa into puppetlabs:master Dec 15, 2015
@igalic
Copy link
Contributor

igalic commented Dec 15, 2015

thanks @KlavsKlavsen, thanks @tphoney for review

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

Successfully merging this pull request may close these issues.

None yet

3 participants