Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TS-4698: Adds an API call to identify WebSocket requests. #822

Closed
wants to merge 1 commit into from
Closed

TS-4698: Adds an API call to identify WebSocket requests. #822

wants to merge 1 commit into from

Conversation

ogoodman
Copy link
Contributor

This makes it possible for plugins to detect incoming WebSocket connections and thereby provide WebSocket termination if desired. An example will be given in a PR addressing TS-4699.

@zwoop zwoop added the TS API label Jul 23, 2016
@zwoop zwoop added this to the 7.0.0 milestone Jul 23, 2016
@zwoop zwoop self-assigned this Jul 23, 2016
@zwoop
Copy link
Contributor

zwoop commented Jul 23, 2016

Cool. Couple of things:

  1. Please email dev@ with a short description for the API proposal. For some details on the process of adding new APIs, see https://cwiki.apache.org/confluence/display/TS/API+Review+Process.

  2. Can you add some documentation as well? It doesn't have to be particularly involved, but perhaps add it to TSHttpTxnIsInternal.en.rst? We prefer to group related APIs together in a single man page type file.

  3. I don't think we have any existing test harnesses or regression tests for web sockets, but something to consider later.

@zwoop
Copy link
Contributor

zwoop commented Jul 23, 2016

[approve ci]

@atsci
Copy link

atsci commented Jul 23, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/477/ for details.

@atsci
Copy link

atsci commented Jul 23, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/374/ for details.

@jpeach jpeach self-assigned this Jul 24, 2016
jpeach
jpeach reviewed Aug 12, 2016
View reviewed changes
proxy/InkAPI.cc
sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);

HttpSM *sm = (HttpSM *)txnp;
return sm->t_state.is_upgrade_request && sm->t_state.upgrade_token_wks == MIME_VALUE_WEBSOCKET;
Copy link
Contributor

@jpeach jpeach Aug 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you test HttpTransact::State::is_websocket? That seems like the correct check in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @ogoodman

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I didn't spot that before. Done.

@jpeach jpeach added the Backport Marked for backport for an LTS patch release label Sep 14, 2016
@zwoop zwoop modified the milestones: 7.1.0, 7.0.0 Sep 15, 2016
@jpeach
Copy link
Contributor

jpeach commented Sep 23, 2016

[approve ci]

@atsci
Copy link

atsci commented Sep 23, 2016

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/756/ for details.

@atsci
Copy link

atsci commented Sep 23, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/860/ for details.

@asfgit asfgit closed this in 9228329 Sep 23, 2016
ericcarlschwartz pushed a commit to ericcarlschwartz/trafficserver that referenced this pull request Sep 29, 2016
@ogoodman @ericcarlschwartz
TS-4698: Adds an API call to identify WebSocket requests.
8a52958 
Add the TSHttpTxnIsWebsocket API to make it possible for plugins
to detect incoming WebSocket connections and thereby provide WebSocket
termination if desired.

This closes apache#822.
@zwoop zwoop modified the milestone: 7.1.0 May 4, 2017
@zwoop zwoop unassigned jpeach and bgaff May 4, 2017
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
H2 connect fix (apache#9834) (apache#822)
1c53839 
These are cherry-picks from asf that has the fix for crash caused by H2 CONNECT.
* Added http connect Autest with proxy verifier (apache#9315)

* Added a HTTP CONNECT test using proxy verifier

* Updated comment

* added proxy-response verification

(cherry picked from commit b5f2023)

* Added Autest for H2 CONNECT and fix a crash triggered by the test (apache#9781)

In apache#9616, @maskit wrote an H2 CONNECT Autest but couldn't include that in the final PR because of a Proxy Verifier issue.
Now that the Proxy Verifier issue is resolved, the Autest is added in this PR(with a few tweaks).

ATS crashes with the new test executing HTTP/2 tunneling traffic. This PR also includes a fix to resolve that.

(cherry picked from commit df7ccfe)

* Update to Proxy Verifier v2.8.1 (apache#9834)

Proxy Verifier v2.8.1 has fixes for the way Proxy Verifier relates to
HTTP/2 CONNECT method request pseudo header fields. This will be helpful
for testing HTTP/2 CONNECT requests.

(cherry picked from commit d5c47a7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zwoop zwoop

Labels
Backport Marked for backport for an LTS patch release TS API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ogoodman @zwoop @atsci @jpeach @bgaff