FISH-9690: adding validation to prevent invalid characters on header values

[breakponchito]

Fix for RFC-9110 regarding invalid characters on header values

Description

This is a fix for a bug reported from the service team regarding to prevent any issue when invalid characters are set on header values based on the RFC-9110

Important Info

Blockers

Testing

New tests

Arquillian test added on the Payara Samples: appserver/tests/payara-samples/samples/rfc-9110/src/test/java/fish/payara/samples/headers/PayaraValidationRFCHeadersIT.java

Testing Performed

Manual testing:
Deploy the following application in Payara 6:
ReproducerJDK11.zip
when the application is deployed open a git bash terminal from windows or use postman to execute a get call for the following URL: http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
for curl use the following command including an invalid header including the following list of invalid characters:

• \x00 \0 : any of those represent NUL
• \x0A \n : any of those represent LF
• \x0D \r : any of those represent CR

example of commands using git bash:
curl -i -v -H "X-MyHeader: \nhello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
curl -i -v -H "X-MyHeader: \rhello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
curl -i -v -H "X-MyHeader: \0hello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
curl -i -v -H "X-MyHeader: \x00hello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
curl -i -v -H "X-MyHeader: \x0Ahello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject
curl -i -v -H "X-MyHeader: \x0Dhello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject

with invalid characters now the response should be a 400 error with specific error code on the server:

server log:

with valid characters the call should return 200
curl -i -v -H "X-MyHeader: hello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/PayaraServletProject

from postman:

Arquillian test execution passing with success:

Testing Environment

Windows 11, azul JDK 11, maven 3.9.5

Documentation

Notes for Reviewers

[breakponchito]

Jenkins test please

[Viii3]

Copyright header needs updating in the pom, other than that LGTM.

[pdudits]

There's no limitation to literal \ characters in RFC9110.

The actual bytes that involve non-visual characters should be rejected (Character.isISOControl), as actuall 0x00 in the byte stream.

[pdudits]

Isn't this too late though? Shouldn'r Grizzly already reject an invalid Request? Based on original report it is indeed the case and Grizzly shouldn't even pass the request to Coyote.

[breakponchito]

yeah at the beginning I thought to add it to the Grizzly side, after checking with the team we thought that Coyote side would be fine. @Pandrex247 any suggestion?

[aubi]

I have a bad news:
curl -i -v -H "X-MyHeader: \nhello" http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/
should pass as there is nothing wrong -- it DOES NOT send the LF as we thought, it really sends '\' 'n' (5c 6e, tried to watch in Wireshark):

0000   00 00 00 00 00 00 00 00 00 00 00 00 86 dd 60 02   ..............`.
0010   37 a3 00 a5 06 40 00 00 00 00 00 00 00 00 00 00   7....@..........
0020   00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00   ................
0030   00 00 00 00 00 01 b3 b6 1f 90 5e ac 46 00 e5 fe   ..........^.F...
0040   1c 63 80 18 02 00 00 ad 00 00 01 01 08 0a 74 37   .c............t7
0050   80 d8 74 37 80 d8 47 45 54 20 2f 50 61 79 61 72   ..t7..GET /Payar
0060   61 53 65 72 76 6c 65 74 50 72 6f 6a 65 63 74 2d   aServletProject-
0070   31 2e 30 2d 53 4e 41 50 53 48 4f 54 2f 20 48 54   1.0-SNAPSHOT/ HT
0080   54 50 2f 31 2e 31 0d 0a 48 6f 73 74 3a 20 6c 6f   TP/1.1..Host: lo
0090   63 61 6c 68 6f 73 74 3a 38 30 38 30 0d 0a 55 73   calhost:8080..Us
00a0   65 72 2d 41 67 65 6e 74 3a 20 63 75 72 6c 2f 38   er-Agent: curl/8
00b0   2e 31 30 2e 31 0d 0a 41 63 63 65 70 74 3a 20 2a   .10.1..Accept: *
00c0   2f 2a 0d 0a 58 2d 4d 79 48 65 61 64 65 72 3a 20   /*..X-MyHeader: 
00d0   5c 6e 68 65 6c 6c 6f 0d 0a 0d 0a                  \nhello....

[aubi]

Ah, there is a way:

curl -i -v -H $'X-MyHeader: \nhello' http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/`

Note the $ char.

00b0   2e 31 30 2e 31 0d 0a 41 63 63 65 70 74 3a 20 2a   .10.1..Accept: *
00c0   2f 2a 0d 0a 58 2d 4d 79 48 65 61 64 65 72 3a 20   /*..X-MyHeader: 
00d0   0a 68 65 6c 6c 6f 0d 0a 0d 0a                     .hello....

Sending zero breaks curl, it doesn't send the header at all :-)

curl -i -v -H $'X-MyHeader: \0hello' http://localhost:8080/PayaraServletProject-1.0-SNAPSHOT/

0090   63 61 6c 68 6f 73 74 3a 38 30 38 30 0d 0a 55 73   calhost:8080..Us
00a0   65 72 2d 41 67 65 6e 74 3a 20 63 75 72 6c 2f 38   er-Agent: curl/8
00b0   2e 31 30 2e 31 0d 0a 41 63 63 65 70 74 3a 20 2a   .10.1..Accept: *
00c0   2f 2a 0d 0a 0d 0a                                 /*....

As expected, if I put \r\n, the server sees it as two separate headers and works normally:

curl -i -v -H $'X-MyHeader: x\r\nX-MyHeader2: y' http://localhost:8080/

* Host localhost:8080 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:8080...
* Connected to localhost (::1) port 8080
* using HTTP/1.x
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.10.1
> Accept: */*
> X-MyHeader: x
> X-MyHeader2: y
> 
* Request completely sent off
< HTTP/1.1 200 OK

[aubi]

The current version refuses \ n, which is OK; this should be allowed.

Please, try the implementation with the $'X-MyHeader: \nhello' version of the curl test.