http_parser: equality check seems like it should be an assignment?

[kevmoo]

sdk/sdk/lib/_http/http_parser.dart

Line 657 in 159bbb6

_messageType == _MessageType.RESPONSE;

Lints are NICE!

[kevmoo]

CC @lrhn and/or @brianquinlan ?

[mnordine]

Why not use unnecessary_statements lint? Seems dangerous not to?

[kevmoo]

@mnordine – exactly how I found it. It should be enabled for the SDK!

[Nishthajain7]

Can i take this issue up?

[lrhn]

That does look like a bug.
The _messageType is never assigned the value _MessageType.RESPONSE, so the three other tests of _messageType == _MessageType.RESPONSE also cannot trigger.

The code is 12 years old, and nobody has ever noticed, so it's probably not a very serious bug.

I'm a little vary about changing it now, without first figuring out why it hasn't been a problem.
(Is the default behavior always the same as for _MessageType.RESPONSE? If so, the fix should be a no-op.
Is there no otherwise valid header where it makes a difference? If so, it may just change an error message.
Have code adapted to the bug, and would be broken if we fixed it?
Can we even write a test that exhibits the bug?)

[kevmoo]

@lrhn – you ask amazing questions. 😁

[Nishthajain7]

Is the default behavior always the same as for _MessageType.RESPONSE?

Yess. In the current implementation, _messageType is initialized to UNDETERMINED (which is 0) but _MessageType.RESPONSE is also set to 0. So _messageType already has the correct value by default! So accidentally, the check works but logically, we should change it to something like

class _MessageType {
  static const int UNDETERMINED = 0;
  static const int REQUEST = 1;
  static const int RESPONSE = 2;  // Change from 0 to 2
}

There would be some failures due to this but I can work on them

[Nishthajain7]

@lrhn