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

HTTPServerResponse.writeJsonBody causes assertion #788

Closed
japplegame opened this Issue Aug 22, 2014 · 1 comment

Comments

Projects
None yet
2 participants
@japplegame
Contributor

japplegame commented Aug 22, 2014

HTTPServerResponse.writeJsonBody causes assertion if compression used.

Test-case:

import vibe.d;

shared static this() {
    auto settings = new HTTPServerSettings;
    settings.port = 8080;
    settings.bindAddresses = ["127.0.0.1"];
    settings.useCompressionIfPossible = true; // compression used

    auto router = new URLRouter;
    router.get("/", (request, response) {
        struct Result {
            string data;
        }
        response.writeJsonBody(Result("test"));
    });

    listenHTTP(settings, router);
}

The reason is incompatibility between compression and non-chunked encoding.
writeJsonBody() adds Content-Length header (see code), but bodyWriter removes it because compression used (see code). Because of this, body writer uses ChunkedOutputStream instead CountingOutputStream, which leads to assertion at this line.

@japplegame japplegame changed the title from `HTTPServerResponse.writeJsonBody` causes assertion to HTTPServerResponse.writeJsonBody causes assertion Aug 22, 2014

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Aug 22, 2014

Member

Hmm, looks like the assertion has to go. There was actually an issue there (one of the output ranges missing a put(char) overload caused too many bytes to be written), so that's a bit unfortunate, but the only other way would be to install another counting wrapper stream and I'm not sure if adding more overhead just for an assertion is a good idea.

Member

s-ludwig commented Aug 22, 2014

Hmm, looks like the assertion has to go. There was actually an issue there (one of the output ranges missing a put(char) overload caused too many bytes to be written), so that's a bit unfortunate, but the only other way would be to install another counting wrapper stream and I'm not sure if adding more overhead just for an assertion is a good idea.

@s-ludwig s-ludwig closed this in e806708 Aug 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment