Daniel Bevenius

Clarifying the that a null String is returned by using @{code}

Updating allowNullOrigin to return 'null' instead of '*'.

Motivation:

Currently the way a 'null' origin, a request that most often indicated

that the request is coming from a file on the local file system, is

handled is incorrect. We are currently returning a wildcard origin '*'

but should be returning 'null' for the 'Access-Control-Allow-Origin'

which is valid according to the specification [1].

Modifications:

Updated CorsHandler to add a 'null' origin instead of the '*' origin in

the case the request origin is 'null.

Result:

All test pass and the CORS example as does the cors.html example if you

try to serve it by opening the file directly in a web browser.

[1]

https://www.w3.org/TR/cors/#access-control-allow-origin-response-header

Add logging configuration to pom.xml

Motivation:

Currently the default log level when running tests is debug. When

running the build on the CI server it might be nice to avoid this debug

level and allow for the level to be configured.

Modifications:

Added a logback-test.xml configuration that has been added to the

common module. This allows for the logLevel to be configured.

The default level will still be debug.

Result:

The log level can now be configured from the command line:

$ mvn test -DlogLevel=error

    • -0
    • +11
    /common/src/test/resources/logback-test.xml
Add logging configuration to pom.xml

Motivation:

Currently the default log level when running tests is debug. When

running the build on the CI server it might be nice to avoid this debug

level and allow for the level to be configured.

Modifications:

Added a logback-test.xml configuration that has been added to the

common module. This allows for the logLevel to be configured.

The default level will still be debug.

Result:

The log level can now be configured from the command line:

$ mvn test -DlogLevel=error

    • -0
    • +11
    /common/src/test/resources/logback-test.xml
When null origin is supported then credentials header must not be set.

Motivation:

Currently CORS can be configured to support a 'null' origin, which can

be set by a browser if a resources is loaded from the local file system.

When this is done 'Access-Control-Allow-Origin' will be set to "*" (any

origin). There is also a configuration option to allow credentials being

sent from the client (cookies, basic HTTP Authentication, client side

SSL). This is indicated by the response header

'Access-Control-Allow-Credentials' being set to true. When this is set

to true, the "*" origin is not valid as the value of

'Access-Control-Allow-Origin' and a browser will reject the request:

http://www.w3.org/TR/cors/#resource-requests

Modifications:

Updated CorsHandler's setAllowCredentials to check the origin and if it

is "*" then it will not add the 'Access-Control-Allow-Credentials'

header.

Result:

Is is possible to have a client send a 'null' origin, and at the same

time have configured the CORS to support that and to allow credentials

in that combination.

Conflicts:

codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java

When null origin is supported then credentials header must not be set.

Motivation:

Currently CORS can be configured to support a 'null' origin, which can

be set by a browser if a resources is loaded from the local file system.

When this is done 'Access-Control-Allow-Origin' will be set to "*" (any

origin). There is also a configuration option to allow credentials being

sent from the client (cookies, basic HTTP Authentication, client side

SSL). This is indicated by the response header

'Access-Control-Allow-Credentials' being set to true. When this is set

to true, the "*" origin is not valid as the value of

'Access-Control-Allow-Origin' and a browser will reject the request:

http://www.w3.org/TR/cors/#resource-requests

Modifications:

Updated CorsHandler's setAllowCredentials to check the origin and if it

is "*" then it will not add the 'Access-Control-Allow-Credentials'

header.

Result:

Is is possible to have a client send a 'null' origin, and at the same

time have configured the CORS to support that and to allow credentials

in that combination.

Conflicts:

codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java

When null origin is supported then credentials header must not be set.

Motivation:

Currently CORS can be configured to support a 'null' origin, which can

be set by a browser if a resources is loaded from the local file system.

When this is done 'Access-Control-Allow-Origin' will be set to "*" (any

origin). There is also a configuration option to allow credentials being

sent from the client (cookies, basic HTTP Authentication, client side

SSL). This is indicated by the response header

'Access-Control-Allow-Credentials' being set to true. When this is set

to true, the "*" origin is not valid as the value of

'Access-Control-Allow-Origin' and a browser will reject the request:

http://www.w3.org/TR/cors/#resource-requests

Modifications:

Updated CorsHandler's setAllowCredentials to check the origin and if it

is "*" then it will not add the 'Access-Control-Allow-Credentials'

header.

Result:

Is is possible to have a client send a 'null' origin, and at the same

time have configured the CORS to support that and to allow credentials

in that combination.

Suggestion for supporting single header fields.

Motivation:

At the moment if you want to return a HTTP header containing multiple

values you have to set/add that header once with the values wanted. If

you used set/add with an array/iterable multiple HTTP header fields will

be returned in the response.

Note, that this is indeed a suggestion and additional work and tests

should be added. This is mainly to bring up a discussion.

Modifications:

Added a flag to specify that when multiple values exist for a single

HTTP header then add them as a comma separated string.

In addition added a method to StringUtil to help escape comma separated

value charsequences.

Result:

Allows for responses to be smaller.

Conflicts:

codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java

codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java

codec/src/main/java/io/netty/handler/codec/DefaultTextHeaders.java

codec/src/test/java/io/netty/handler/codec/DefaultTextHeadersTest.java

Suggestion for supporting single header fields.

Motivation:

At the moment if you want to return a HTTP header containing multiple

values you have to set/add that header once with the values wanted. If

you used set/add with an array/iterable multiple HTTP header fields will

be returned in the response.

Note, that this is indeed a suggestion and additional work and tests

should be added. This is mainly to bring up a discussion.

Modifications:

Added a flag to specify that when multiple values exist for a single

HTTP header then add them as a comma separated string.

In addition added a method to StringUtil to help escape comma separated

value charsequences.

Result:

Allows for responses to be smaller.

Fixing minor typo in FastThreadLocal javadoc.

Fixing minor typo in FastThreadLocal javadoc.

Fixing minor typo in FastThreadLocal javadoc.

Adding a propagateSettings flag to InboundHttp2ToHttpAdapter.

Motivation:

When DefaultHttp2FrameReader has read a settings frame, the settings

will be passed along the pipeline. This allows a client to hold off

sending data until it has received a settings frame. But for a server it

will always have received a settings frame and the usefulness of this

forwarding of settings is less useful. This also causes a debug message

to be logged on the server side if there is no channel handler to handle

the settings:

[nioEventLoopGroup-1-1] DEBUG io.netty.channel.DefaultChannelPipeline -

Discarded inbound message {INITIAL_WINDOW_SIZE=131072,

MAX_FRAME_SIZE=16384} that reached at the tail of the pipeline. Please

check your pipeline configuration.

Modifications:

Added a builder for the InboundHttp2ToHttpAdapter and

InboundHttp2PriortyAdapter and a new parameter named 'propagateSettings'

to their constructors.

Result:

It is now possible to control whether settings should be passed along

the pipeline or not.

Adding codeAsText to HttpResponseStatus.

Motivation:

I found myself writing AsciiString constants in my code for

response statuses and thought that perhaps it might be nice to have

them defined by Netty instead.

Modifications:

Adding codeAsText to HttpResponseStatus that returns the status code as

AsciiText.

In addition, added the 421 Misdirected Request response code from

https://tools.ietf.org/html/draft-ietf-httpbis-http2-15#section-9.1.2

This response header was renamed in draft 15:

https://tools.ietf.org/html/draft-ietf-httpbis-http2-15#appendix-A.1

But the code itself was not changed, and I thought using the latest would

be better.

Result:

It is now possible to specify a status like this:

new DefaultHttp2Headers().status(HttpResponseStatus.OK.codeAsText());

Adding codeAsText to HttpResponseStatus.

Motivation:

I found myself writing AsciiString constants in my code for

response statuses and thought that perhaps it might be nice to have

them defined by Netty instead.

Modifications:

Adding codeAsText to HttpResponseStatus that returns the status code as

AsciiText.

In addition, added the 421 Misdirected Request response code from

https://tools.ietf.org/html/draft-ietf-httpbis-http2-15#section-9.1.2

This response header was renamed in draft 15:

https://tools.ietf.org/html/draft-ietf-httpbis-http2-15#appendix-A.1

But the code itself was not changed, and I thought using the latest would

be better.

Result:

It is now possible to specify a status like this:

new DefaultHttp2Headers().status(HttpResponseStatus.OK.codeAsText());

Allow DefaultHttp2Headers to be forced to lowercase.

Motivation:

I came across an issue when I was adding/setting headers and mistakenly

used an upper case header name. When using the http2 example that ships

with Netty this was not an issue. But when working with a browser that

supports http2, in my case I was using Firefox Nightly, I'm guessing

that it interprets the response as invalid in accordance with the

specifiction

https://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.2

"However, header field names MUST be converted to lowercase prior

to their encoding in HTTP/2. A request or response containing

uppercase header field names MUST be treated as malformed"

This PR suggests converting to lowercase to be the default.

Modifications:

Added a no-args constructor that defaults to forcing the key/name to

lowercase, and providing a second constructor to override this behaviour

if desired.

Result:

It is now possible to specify a header like this:

Http2Headers headers = new DefaultHttp2Headers(true)

.status(new AsciiString("200"))

.set(new AsciiString("Testing-Uppercase"), new AsciiString("some value"));

And the header written to the client will then become:

testing-uppercase:"some value"

Add logLevel property to enable different log levels for the examples.

Motivation:

When running the examples using the provided run-examples.sh script the

log level is 'info' level. It can be handy to be able to configure a

different level, for example 'debug', while learning and trying out the

the examples.

Modifications:

Added a dependency to logback-classic to the examples pom.xml, and also

added a logback configuration file. The log level can be configured by

setting the 'logLevel' system property, and if that property is not set

the default will be 'info' level.

The run-examples.sh was updated to show an example of using the system

property to set the log level to 'debug'

Result:

It is now possible to turn on debug logging by settnig a system property

on the command line.

    • -0
    • +11
    /example/src/main/resources/logback.xml
Add logLevel property to enable different log levels for the examples.

Motivation:

When running the examples using the provided run-examples.sh script the

log level is 'info' level. It can be handy to be able to configure a

different level, for example 'debug', while learning and trying out the

the examples.

Modifications:

Added a dependency to logback-classic to the examples pom.xml, and also

added a logback configuration file. The log level can be configured by

setting the 'logLevel' system property, and if that property is not set

the default will be 'info' level.

The run-examples.sh was updated to show an example of using the system

property to set the log level to 'debug'

Result:

It is now possible to turn on debug logging by settnig a system property

on the command line.

    • -0
    • +11
    /example/src/main/resources/logback.xml
Add logLevel property to enable different log levels for the examples.

Motivation:

When running the examples using the provided run-examples.sh script the

log level is 'info' level. It can be handy to be able to configure a

different level, for example 'debug', while learning and trying out the

the examples.

Modifications:

Added a dependency to logback-classic to the examples pom.xml, and also

added a logback configuration file. The log level can be configured by

setting the 'logLevel' system property, and if that property is not set

the default will be 'info' level.

The run-examples.sh was updated to show an example of using the system

property to set the log level to 'debug'

Result:

It is now possible to turn on debug logging by settnig a system property

on the command line.

    • -0
    • +11
    /example/src/main/resources/logback.xml
Make SslProvider.JDK the default provider for http2 examples using ALPN.

Motivation:

When running the http2 example no SslProvider is specified when calling

SslContext.newServerContext. This may lead to the provider being

determined depending on the availabilty of OpenSsl. But as far as I can

tell the OpenSslServerContext does not support APLN, which is the

protocol configured in the example.

This produces the following error when running the example:

Exception in thread "main" java.lang.UnsupportedOperationException:

OpenSSL provider does not support ALPN protocol

io.netty.handler.ssl.OpenSslServerContext.toNegotiator(OpenSslServerContext.java:391)

io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:117)

io.netty.handler.ssl.SslContext.newServerContext(SslContext.java:238)

io.netty.handler.ssl.SslContext.newServerContext(SslContext.java:184)

io.netty.handler.ssl.SslContext.newServerContext(SslContext.java:124)

io.netty.example.http2.server.Http2Server.main(Http2Server.java:51)

Modifications:

Force SslProvider.JDK when creating the SslContext since the

example is using APLN.

Result:

There is no longer an error if OpenSsl is supported on the platform in

use.

Minor corrections to Http2 example javadocs.

Motivation:

There are a few very minor issues in the Http2 examples javadoc and

since I don't think that these javadocs are published this is very much

optional to include.

Modifications:

Updated the @see according to [1] to avoid warning when generating

javadocs.

Result:

No warning when generating javadocs.

[1] http://docs.oracle.com/javase/1.5.0/docs/tooldocs/windows/javadoc.html#@see

Minor update to Http2LifecycleManager#closeLocalSide javadoc

Modifications:

Changed "remote" to "local" in javadoc for closeLocalSide.

CorsHandler should release HttpRequest after processing preflight/error.

Motivation:

Currently, when the CorsHandler processes a preflight request, or

respondes with an 403 Forbidden using the short-curcuit option, the

HttpRequest is not released which leads to a buffer leak.

Modifications:

Releasing the HttpRequest when done processing a preflight request or

responding with an 403.

Result:

Using the CorsHandler will not cause buffer leaks.

CorsHandler should release HttpRequest after processing preflight/error.

Motivation:

Currently, when the CorsHandler processes a preflight request, or

respondes with an 403 Forbidden using the short-curcuit option, the

HttpRequest is not released which leads to a buffer leak.

Modifications:

Releasing the HttpRequest when done processing a preflight request or

responding with an 403.

Result:

Using the CorsHandler will not cause buffer leaks.

CorsHandler should release HttpRequest after processing preflight/error.

Motivation:

Currently, when the CorsHandler processes a preflight request, or

respondes with an 403 Forbidden using the short-curcuit option, the

HttpRequest is not released which leads to a buffer leak.

Modifications:

Releasing the HttpRequest when done processing a preflight request or

responding with an 403.

Result:

Using the CorsHandler will not cause buffer leaks.

Adding payload to the Http2Client example.

Modifications:

When trying out the Http2Client example I noticed that adding a request

payload would not cause a data frame to written. This seems to be

because writeHeaders completes the promise, and then the writeData

call ends up in FlowControlWriter.writeFrame, isDone is true and

the data released and the call aborted and returned.

Adding a new promise for the writeData method allows a data frame to be

written.

Result:

A body/payload can now be sent to the server. The example was updated to

simply echo the payload received back to the calling client.

Changed Http2ClientConnectionHandler to print out the aggregated buffers content

Motivation:

When running the Http2Client the data returned from the server, the

"Hello World" string, is supposed to be printed but instead the following

is displayed:

Received message:

Modifications:

Use the aggregated buffer to print out.

Result:

The example now logs the correct data sent from the server:

Received message: Hello World

OkResponseHandler should return a FullHttpResponse.

Motivation:

Currently OkResponseHandler returns a DefaultHttpResponse which is not

correct and it should be returning complete http response.

Modifications:

Updated OkResponseHandler to return an instance of

DefaultFullHttpResponse.

Result:

It is not possible to add compression to the example without getting any

errors.

OkResponseHandler should return a FullHttpResponse.

Motivation:

Currently OkResponseHandler returns a DefaultHttpResponse which is not

correct and it should be returning complete http response.

Modifications:

Updated OkResponseHandler to return an instance of

DefaultFullHttpResponse.

Result:

It is not possible to add compression to the example without getting any

errors.

OkResponseHandler should return a FullHttpResponse.

Motivation:

Currently OkResponseHandler returns a DefaultHttpResponse which is not

correct and it should be returning complete http response.

Modifications:

Updated OkResponseHandler to return an instance of

DefaultFullHttpResponse.

Result:

It is not possible to add compression to the example without getting any

errors.