[TT-13257] Fix websocket upgrade with multiple values in Connection header - Githubissues

TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols

Other

9.75k stars 1.09k forks source link

[TT-13257] Fix websocket upgrade with multiple values in Connection header #6619

Open Darkness4 opened 1 month ago

Darkness4 commented 1 month ago

Description

To check for Websocket connection upgrade, replaces the "!=" check with the "not contains" check.

Related Issue

Closes #6449.

How This Has Been Tested

A test case has been added.

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[x] Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

[x] I ensured that the documentation is up to date
[ ] ~I explained why this PR updates go.mod in detail with reasoning why it's required~
[ ] ~I would like a code coverage CI quality gate exception and have explained why~

titpetric commented 1 month ago

Hi,

thank you for the contribution. The approach looks good for the most part, two requests raised from review:

can you use testify/assert in tests (we're standardising on it, but have some fragmentation)
can you please extend the internal/httputil location with a unit test (ideally that's black box)

Thank you :)

Darkness4 commented 1 month ago

@titpetric You can re-review. :slightly_smiling_face: