Bug 65155 - ab: support HTTP/1.1
Summary: ab: support HTTP/1.1
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: support (show other bugs)
Version: 2.4-HEAD
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-22 14:34 UTC by Andrei
Modified: 2021-02-25 13:00 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei 2021-02-22 14:34:46 UTC
Apache Benchmark (ab) currently supports making requests only using HTTP/1.0 ( https://github.com/apache/httpd/blob/trunk/support/ab.c#L1886 )

Some proxies/sidecars used in Kubernetes (e.g. Envoy when installed by Istio) support only HTTP/1.1 (or 2) by default. It would be nice if there was a flag to enable HTTP/1.1 requests in ab.

Is this something that would be acceptable for ab or is it completely out of scope?
Comment 1 Yann Ylavic 2021-02-24 21:39:25 UTC
(In reply to Andrei from comment #0)
> Is this something that would be acceptable for ab or is it completely out of
> scope?

Sure, there could be for instance a command line "-1" option to control this.
Would you propose a patch?
Comment 2 Andrei 2021-02-25 10:13:18 UTC
I've just tried now to create a PR ( https://github.com/apache/httpd/pull/173 ) but the change fails because C89/C90 style is (still) being used (e.g. https://travis-ci.com/github/apache/httpd/jobs/486139913 )

ab.c:1874:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
     const char *const http_version = http_spec == HTTP_1_1 ? "HTTP/1.1" : "HTTP/1.0";
     ^~~~~


I would have preferred to use a const pointer, but that would mean to move all three initializations to the top of the function. Which solution is preferred?

* move definitions and initialization at the top of the file
* define at top, initialize just before apr_snprintf 
* inline again the initialization as arguments to the apr_snprintf function (I don't find this easily readable)
Comment 3 Yann Ylavic 2021-02-25 10:41:12 UTC
(In reply to Andrei from comment #2)
> Which solution is
> preferred?
> 
> * move definitions and initialization at the top of the file
> * define at top, initialize just before apr_snprintf 
> * inline again the initialization as arguments to the apr_snprintf function

Move the const definitions with initializations at the top of the test() function?
It seems that the initializations depend on globals only so there's no previous initialization in in the test() function itself which prevents that, unless I'm missing something..
Comment 4 Joe Orton 2021-02-25 12:18:29 UTC
If it's going to advertise HTTP/1.1 support, ab also needs to support HTTP/1.1 message syntax, e.g. chunked transfer-coding, which it doesn't currently, so surely this isn't as simple as changing the request-line.
Comment 5 Andrei 2021-02-25 13:00:47 UTC
(In reply to Yann Ylavic from comment #3)
> Move the const definitions with initializations at the top of the test()
> function?

Yes, at the top of the function, not the method.

(In reply to Joe Orton from comment #4)
> If it's going to advertise HTTP/1.1 support, ab also needs to support
> HTTP/1.1 message syntax, e.g. chunked transfer-coding, which it doesn't
> currently, so surely this isn't as simple as changing the request-line.

Yes, you're right. I was just thinking of my (simple) use case... :|