dylan-lang / http

HTTP server and client for Dylan
https://opendylan.org/package/http
MIT License
22 stars 11 forks source link

Invoke specialized write for <response> #61

Closed kibook closed 5 years ago

kibook commented 10 years ago

The <response> type has a specialized version of write and write-element defined in server/core/response.dylan for <response> and <byte-string>, but copy-to-end here instead called a more generic version for <stream> and <sequence>.

This meant the <response> was being written to as a normal stream, instead of using the maybe-send-chunk method and writing to response.response-stream.

Coercing the parameters to the types expected by the write and write-element methods in server/core/response.dylan fixes an issue with <directory-resource> serving static files.

cgay commented 10 years ago

The write method called should be determined by the run-time types of both arguments. stream must already be a <response>, otherwise as(<response>, stream) would err. I surmise that buffer is not a <byte-string> and that's why it was failing to call write(<response>, <byte-string>). Could you verify that?

Rather than copy the buffer with as(<byte-string>, buffer) I think it would be better to define a write(<response>, <sequence>) method next to the write(<response>, <byte-string>) method. (Keep the original method so that byte strings remain relatively fast.)

cgay commented 10 years ago

It's a good idea to have a more general method there because this is an easy mistake to make and it's better to work and be slow than to pretend to work.

kibook commented 10 years ago

Yes, the buffer used in copy-to-end was just <sequence>, which was why the write method in server/core/response.dylan wasn't being called.

cgay commented 10 years ago

kibook, it would be good if you could remove the reverted change from this pull request before we merge it. Otherwise it looks good. Thanks!