-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

All,

While reading this BZ issue[1], I decided to read the code referred to
by Michael Osipov in Http11OutputBuffer.java.

The method is sendStatus and looks roughly like this:

    write(Constants.HTTP_11_BYTES);
    headerBuffer.put(Constants.SP);
    write(statuscode);
    headerBuffer.put(Constants.SP);
    write(reasonPhrase);           // Only in Tomcat < 9.0
    headerBuffer.put(Constants.CR).put(Constants.LF);

headerBuffer is a protected final ByteBuffer member and write() is an
overloaded member method where only the following implementation is used
:

  public void write(byte[])

The write() method always checks to see if headerBuffer is big enough
to hold the byte array being passed as an argument, and throws a
HeadersTooLargeException if the byte buffer would overflow. If no
overflow will occur, write() calls headerBuffer.put().

headerBuffer.put will throw a BufferOverflowException if the buffer
overflows.

This code is inconsistent in the way that it handles access to the
headerBuffer (ByteBuffer): one access strategy is to just shove things
into the buffer (headerBuffer.put) and another is to perform a "safe"
write to the buffer. These calls are interleaved.

Reading only the code for sendStatus() -- that is, without reading the
body of write() -- it's not clear at all that calls to write() and
headerBuffer.put() are both modifying the same buffer.

In the specific case of Tomcat < 9.0, the exact length of the reason
phrase will determine whether you receive a BufferOverflowException or
a HeadersTooLargeException. I think you should consistently receive
one or the other of these two error conditions because they mean
exactly the same thing: you overflowed the response buffer before the
headers were completely written.

Is there a particular reason not to consistently call write() in
sendStatus() instead of mixing these two types of calls?

If performance is a consideration, then most of the calls to write()
should probably be calls to headerBuffer.put() because we can be
(reasonably?) sure that writing "HTTP/1.1 " to the output buffer isn't
going to overflow the buffer. Likewise with the bytes for the status
code. With an omitted reason phrase, even the trailing SP CR LF can be
collapsed into a single byte[] and written a single time to the
ByteBuffer, instead of individually as the current code does.

If there is a reason to be inconsistent, let's state it and be
consistently inconsistent. If there is no such reason, let's be
consistent and either always call write() or always call
headerBuffer.put().

Thoughts?

Thanks,
- -chris

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=61982
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQJRBAEBCAA7FiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlpX70cdHGNocmlzQGNo
cmlzdG9waGVyc2NodWx0ei5uZXQACgkQHPApP6U8pFjoRQ//TR13uVN6fwMs3COf
PRqE+51hK+nb15yP+PtHPCR73hf8c64A/nM5CZENpMcbUVrI2tr8HdMD8tPo1a5P
T6SheiCyENrBXk7qpcHcxcy9oCD7jvrbuYBtCdDJhcBVgwn1sqONtMQrl03Ioo/u
6bahOIPQrQEMpjwV4BBuWREA1HD+6Pn432DKbsMTJzIyUCjLPIBtBQhotSdhgYJb
zOsntQzWvI7u2CFogrER6/Vm2E1gtBfs5qH/BDe3h3coIjgCnEcgwAQbfdobZ7eb
IeoG0bC5HTR1inNsnt/4T+hIVxH43asLa9EcebQe+PnwS0wm+HenAgULaphR+9Sn
bYYPb31BVsi75P/v9WTouJ18p0NaWYgDA/XVO6iX+oLFhhIt0rxInkBl+My2rK+H
bnPebPdwvt/ZY9Si+qTqUccMJKanVG13AH8AvKejfntbWbEjl+vrWWRXSS+PMYsh
xNDujEaUcoiVUkoKWX96EKfhFiWEhKbnLNuHjKu7ckhhPqgefqN4l43USiuDLnFv
1s48t7/DR0/X6DSOEq8JyXmcP4twroVFTQ+E+8Bg2igaDtUIpy/QfSnWtO0mDFSN
J2AfVOvm/WibhWZBDLZaQ5iXHnUrtEyAoiMfOeUza6H0Km6LB192X8EjonFD421I
4WBW6Nfb5H3xqe60YniG6Qkb+0s=
=Doi6
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to