pkg/shelf: Fixed logic for setting Server header in `shelf_io`

pkg/shelf: Fixed logic for setting Server header in `shelf_io`

BUG= https://code.google.com/p/dart/issues/detail?id=18737
R=nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36725

[kevmoo, 2014-05-15 03:21:28]

The code that shipped was dumb. Special casing the server header seems
superfluous

[nweiz, 2014-05-15 22:21:31]

https://codereview.chromium.org/281353004/diff/1/pkg/shelf/lib/shelf_io.dart
File pkg/shelf/lib/shelf_io.dart (left):

https://codereview.chromium.org/281353004/diff/1/pkg/shelf/lib/shelf_io.dart#...
pkg/shelf/lib/shelf_io.dart:93: }
I still like the intent of this code, even if it didn't work correctly. Can you
figure out how dart:io's server computes its default header and use the same
logic here?

[kevmoo, 2014-05-25 01:17:13]

PTAL


I thought some more about this. I want to improve the logic here, but I want to
make sure there is a way to opt-out of any default behavior of shelf_io.

The design we discussed allows changing the default behavior, but not saying "no
header" - something I think reasonable.

Let's land this CL (which fixes the current weird/broken behavior) and discuss
alternatives later.

[nweiz, 2014-05-27 19:46:26]

On 2014/05/25 01:17:13, kevmoo wrote:
> PTAL
> 
> 
> I thought some more about this. I want to improve the logic here, but I want
to
> make sure there is a way to opt-out of any default behavior of shelf_io.
> 
> The design we discussed allows changing the default behavior, but not saying
"no
> header" - something I think reasonable.
> 
> Let's land this CL (which fixes the current weird/broken behavior) and discuss
> alternatives later.

I'd much rather add a server header for now and find a way to disable it later.
That's what all servers in other languages do and I don't see a good reason to
deviate from that behavior. The current behavior, although broken, is closer to
the standard than the new behavior introduced by this CL.

[kevmoo, 2014-05-27 22:15:50]

PTAL

[nweiz, 2014-05-27 23:08:05]

lgtm

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/CHANGELOG.md
File pkg/shelf/CHANGELOG.md (right):

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/CHANGELOG.md#n...
pkg/shelf/CHANGELOG.md:4: * Fixed logic for setting Server header in `shelf_io`
End with a period, add a newline between this and the previous bullet.

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:293: expect(response.headers,
containsPair(HttpHeaders.SERVER, 'dart:io with Shelf'));
Long line

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:297: test('deferres to response header when',
() {
"deferres" -> "defers", "response header when" -> "header in response"

[kevmoo, 2014-05-28 00:44:04]

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/CHANGELOG.md
File pkg/shelf/CHANGELOG.md (right):

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/CHANGELOG.md#n...
pkg/shelf/CHANGELOG.md:4: * Fixed logic for setting Server header in `shelf_io`
On 2014/05/27 23:08:05, nweiz wrote:
> End with a period, add a newline between this and the previous bullet.

Done.

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:293: expect(response.headers,
containsPair(HttpHeaders.SERVER, 'dart:io with Shelf'));
On 2014/05/27 23:08:05, nweiz wrote:
> Long line

Done.

https://codereview.chromium.org/281353004/diff/50001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:297: test('deferres to response header when',
() {
On 2014/05/27 23:08:05, nweiz wrote:
> "deferres" -> "defers", "response header when" -> "header in response"

Done.

[kevmoo, 2014-05-28 00:44:22]

Committed patchset #5 manually as r36725 (presubmit successful).