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

Add LimitRequestFields parameter in main configuration #1742

Merged
merged 8 commits into from Jan 3, 2018
Merged

Add LimitRequestFields parameter in main configuration #1742

merged 8 commits into from Jan 3, 2018

Conversation

geekix
Copy link

@geekix geekix commented Dec 19, 2017

Hello,

I need to configure this parameter : LimitRequestFields (http://httpd.apache.org/docs/current/mod/core.html#limitrequestfields) but it is not managed by your module yet.

So I took the liberty to add this parameter in the init.pp file (with a value at '100', which is the default value in apache) and in the httpd.conf template.

Successfully tested on Debian 8.9 and apache 2.4.10.

Thanks

@geekix
Copy link
Author

geekix commented Dec 19, 2017

I took a look at the logs but I don't get why the travis tests fail.

@david22swan
Copy link
Member

Please add docs to reflect your changes and tests to insure that it works correctly. Also the test fail is unrelated to your code and part of a separate problem.

@geekix
Copy link
Author

geekix commented Dec 21, 2017

Hello,

What kind of test do you need?

@david22swan
Copy link
Member

Just a simple test that covers it's usage. One that utilises your change and then confirms that it worked as it should.

@geekix
Copy link
Author

geekix commented Dec 21, 2017

Hello,

I configure my apache server with the following value :

user@server ~ :) # grep LimitRequestFields /etc/apache2/apache2.conf
LimitRequestFields 4

This means that the server will only accept a maximum of 4 headers per HTTP request. Then I used apache benchmark to send HTTP requests, here is the result with only one header :

user@server ~ :) #  ab -n1 -c1 -H "header1:test" -v 2 http://127.0.0.1/
This is ApacheBench, Version 2.3 <$Revision: 1604373 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking 127.0.0.1 (be patient)...INFO: GET header ==
---
GET / HTTP/1.0
header1:test
Host: 127.0.0.1
User-Agent: ApacheBench/2.3
Accept: */*


---
LOG: header received:
HTTP/1.1 200 OK
Date: Thu, 21 Dec 2017 14:44:51 GMT
Server: Apache
Last-Modified: Mon, 04 Dec 2017 10:42:20 GMT
Accept-Ranges: bytes
Content-Length: 589
Vary: Accept-Encoding
Connection: close
Content-Type: text/html

<!doctype html>
<html lang="fr">
<head>
  <meta charset="utf-8">
  <title>It works</title>
</head>
<body>
It works !
</body>
</html>


..done


Server Software:        Apache
Server Hostname:        127.0.0.1
Server Port:            80

Document Path:          /
Document Length:        589 bytes

Concurrency Level:      1
Time taken for tests:   0.001 seconds
Complete requests:      1
Failed requests:        0
Total transferred:      817 bytes
HTML transferred:       589 bytes
Requests per second:    1040.58 [#/sec] (mean)
Time per request:       0.961 [ms] (mean)
Time per request:       0.961 [ms] (mean, across all concurrent requests)
Transfer rate:          830.23 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:     1    1   0.0      1       1
Waiting:        0    0   0.0      0       0
Total:          1    1   0.0      1       1

Then an other request with 5 headers. As you can see, the request returns a 400 HTTP error (HTTP/1.1 400 Bad Request) with this error message : The number of request header fields exceeds this server's limit

user@server ~ :) #  ab -n1 -c1 -H "header1:test" -H "header2:test" -H "header3:test" -H "header4:test" -H "header5:test" -v 2 http://127.0.0.1/
This is ApacheBench, Version 2.3 <$Revision: 1604373 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking 127.0.0.1 (be patient)...INFO: GET header ==
---
GET / HTTP/1.0
header1:test
header2:test
header3:test
header4:test
header5:test
Host: 127.0.0.1
User-Agent: ApacheBench/2.3
Accept: */*


---
LOG: header received:
HTTP/1.1 400 Bad Request
Date: Thu, 21 Dec 2017 14:44:01 GMT
Server: Apache
Content-Length: 290
Connection: close
Content-Type: text/html; charset=iso-8859-1

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>400 Bad Request</title>
</head><body>
<h1>Bad Request</h1>
<p>Your browser sent a request that this server could not understand.<br />
The number of request header fields exceeds this server's limit.</p>
</body></html>

WARNING: Response code not 2xx (400)
..done


Server Software:        Apache
Server Hostname:        127.0.0.1
Server Port:            80

Document Path:          /
Document Length:        290 bytes

Concurrency Level:      1
Time taken for tests:   0.001 seconds
Complete requests:      1
Failed requests:        0
Non-2xx responses:      1
Total transferred:      456 bytes
HTML transferred:       290 bytes
Requests per second:    1207.73 [#/sec] (mean)
Time per request:       0.828 [ms] (mean)
Time per request:       0.828 [ms] (mean, across all concurrent requests)
Transfer rate:          537.82 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:     1    1   0.0      1       1
Waiting:        0    0   0.0      0       0
Total:          1    1   0.0      1       1

@geekix
Copy link
Author

geekix commented Dec 22, 2017

As for the module itself, here is what I configured in my hiera file :

apache::limitreqfields: '120'

And the result on the node :

Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Info: Caching catalog for server@domain.tld
Info: Applying configuration version '208a3f3b10466ed298e9addcdd82162ca2074f5e'
Notice: /Stage[main]/Apache/File[/etc/apache2/apache2.conf]/content:
--- /etc/apache2/apache2.conf   2017-12-22 08:58:48.789589106 +0100
+++ /tmp/puppet-file20171222-35390-dntnjy       2017-12-22 08:59:02.000000000 +0100
@@ -11,7 +11,7 @@
 MaxKeepAliveRequests 100
 KeepAliveTimeout 15
 LimitRequestFieldSize 8190
-LimitRequestFields 4
+LimitRequestFields 120

Notice: /Stage[main]/Apache/File[/etc/apache2/apache2.conf]/content: content changed '{md5}c675a39664137d77257c44da1352c961' to '{md5}029ecf57552a49875bb054554a02a4c1'
Info: /Stage[main]/Apache/File[/etc/apache2/apache2.conf]: Scheduling refresh of Class[Apache::Service]
Info: Class[Apache::Service]: Scheduling refresh of Service[httpd]
Notice: /Stage[main]/Apache::Service/Service[httpd]: Triggered 'refresh' from 1 events```

@geekix
Copy link
Author

geekix commented Jan 2, 2018

Hi there,

Happy new year ^^

Do I need to provide more informations on this subject? Are there still unclear points?

Thanks

@david22swan
Copy link
Member

@geekix Still needs tests to be implemented, simply add one where you set a limit and then test to ensure that you can't go over it.

@geekix
Copy link
Author

geekix commented Jan 2, 2018

Hello,

If you go over my last comments, you will find a test that shows that the limit works. Please tell me if you need more explanations.

@david22swan
Copy link
Member

@geekix I feel that you misunderstand, When I say tests I mean automated ones that are built into the module.

@geekix
Copy link
Author

geekix commented Jan 2, 2018

@david22swan I'm sorry, I don't know about automated tests. Can you please give me a hint?

Thanks

@david22swan
Copy link
Member

@geekix You should take a look at this test file:

https://github.com/puppetlabs/puppetlabs-apache/blob/master/spec/acceptance/apache_parameters_spec.rb

Look at line 388 to 400, it's the test for 'limitreqfieldsize'.
Try to copy what it does.

@geekix
Copy link
Author

geekix commented Jan 2, 2018

I'll take a look, thanks for the advice

@geekix
Copy link
Author

geekix commented Jan 2, 2018

I have added a test in the file apache_parameters_spec.rb. I hope it's what you expect.

@david22swan david22swan merged commit 6f8d5a5 into puppetlabs:master Jan 3, 2018
cegeka-jenkins pushed a commit to cegeka/puppet-apache that referenced this pull request Jul 15, 2020
Add LimitRequestFields parameter in main configuration
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