[BUG] If `deadline` argument to `WriteControl` is `deadline.IsZero()` then 1000 hours (~41 days) are used as unexpected and undocumented fallback

[JannikGM]

Is there an existing issue for this?

• [X] I have searched the existing issues

Current Behavior

If the deadline argument is deadline.IsZero() then 1000 hours are used instead:

https://github.com/gorilla/websocket/blob/d08ee1ad9b8a1bad1b388165f85f7794a604b8e1/conn.go#L448-L456

Expected Behavior

Behaviour should be documented and better design decision has to be made:

Either:

• ignore zero as invalid argument: return errWriteTimeout.
• ignore zero deadline: The maximum possible deadline should be used, not arbitrary value of 1000 hours.
• respect zero as valid deadline: Fail request immediately.

Steps To Reproduce

Call WriteControl with a zero-deadline but fail to respond for ~41 days.

This is rather theoretical, but a non-conformant websocket or bad firewall might lead to this.

Anything else?

Originally reported in https://github.com/gorilla/websocket/issues/841#issue-1814538001. Confirmed as "unexpected" by maintainer in https://github.com/gorilla/websocket/issues/841#issuecomment-1936818588.

In practice probably doesn't affect anybody; it's just weird.

[jaitaiwan]

Champion @JannikGM thank you. I'm taking a look at this now.

[jaitaiwan]

Nothing in the RFC seems to indicate any sane value for this. I don't know of any websocket extensions that need to be accounted for in any changes here.

It appears to me that this code is used in both client and server connections, so this probably is a more important function when it comes to clients being unable to write to the server.

Based on that information I think that the following actions should be taken:

1. The default is documented as a warning
2. Any examples should be updated to include setting this deadline to a sane standard
3. This issue should be left open and marked for a v2 as changing this could be a nasty surprise for implementors that don't know about this