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.

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

stream: Allow sending data in the preread phase #392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shankerwangmiao
Copy link
Contributor

There are cases when some interactions should happen between the client
and the server before a successful preread. This patch enables sending
data to the client in the preread phase. To avoid possible conflict
between the preread callback and filtering callbacks, the data sent
bypasses js_body_filter, but not other filters, if any.

This patch introduces a new field in ngx_stream_js_ctx_t named preread
to indicate if the session is now in the preread phase.

@shankerwangmiao
Copy link
Contributor Author

@xeioex any idea on this patch?

There are cases when some interactions should happen between the client
and the server before a successful preread. This patch enables sending
data to the client in the preread phase. To avoid possible conflict
between the preread callback and filtering callbacks, the data sent
bypasses `js_body_filter`, but not other filters, if any.

This patch introduces a new field in ngx_stream_js_ctx_t named `preread`
to indicate if the session is now in the preread phase.
@xeioex
Copy link
Contributor

xeioex commented Aug 13, 2021

Hi @shankerwangmiao,

We do not think it is good idea to allow any sends in preread phase. Preread phase is designed to have no side effects on the stream (think of idempotent request as analogy).

@shankerwangmiao
Copy link
Contributor Author

But sometimes it is needed to do a simple handshake to read meaningful information out before deciding which upstream backend can be used. That's why I'm suggesting this feature.

@xeioex
Copy link
Contributor

xeioex commented Aug 13, 2021

@shankerwangmiao

But sometimes it is needed to do a simple handshake to read meaningful information out before deciding which upstream backend can be used. That's why I'm suggesting this feature.

Can you elaborate on this? Please provide real world example. The whole approach seems brittle to me.

@shankerwangmiao
Copy link
Contributor Author

A real example is rsync protocol. I need to route rsync requests for different modules to different backend servers, because files are stored separately. Although rsync protocol is rather complex, the handshaking process is simple. When connected, the client and server send their version to each other. When the client receives the version number, it will send the name of the module it want to operate on. The first work the njs script needs to do is to emulate a rsync server and send out a version number in the preread phase. When received, the module name will be put into a variable, which will then be mapped to a certain backend. The second work is to filter the version number sent by the backend rsync server and prevent it from being sent to the client. After that, the njs script will detach the event handler and let the connection go.

@shankerwangmiao
Copy link
Contributor Author

Here is an implementation of such rsync proxy using njs:

https://gist.github.com/shankerwangmiao/ffe48d51eef5b7178a442ba4c32568a2

In the above configuration, for example, when the client execute rsync rsync://ip.addr:12345/debian, the connection will be forwarded to mirrors.bfsu.edu.cn:873.

The njs script is now actually running on our server.

@thresheek thresheek closed this Sep 23, 2021
@shankerwangmiao
Copy link
Contributor Author

Hi,

According to readme of njs

Please ... send patches ... via Github:

I wonder if this is the right place to send patches to njs?

@thresheek
Copy link
Member

Sorry @shankerwangmiao was an error in our automation, reopening again.

@thresheek thresheek reopened this Sep 23, 2021
@shankerwangmiao
Copy link
Contributor Author

ping @xeioex

2 similar comments
@shankerwangmiao
Copy link
Contributor Author

ping @xeioex

@shankerwangmiao
Copy link
Contributor Author

ping @xeioex

@fuweichin
Copy link

fuweichin commented Jun 29, 2022

It's interesting that Chrome 103 supports HTTP status code 103 (Early Hints).

Please also consider sending status 1xx (more specifically, status 103) and headers (like 'Link') in the prereadprocessing phase in ngx_http_js_module, for HTTP/1.1 and/or HTTP/2.

And I just get the 103 Early Hints working in Node.js Express and Java Tomcat, but noway in Nginx njs. Here are the code screenshots, waiting for njs to launch such feature:
nodejs-early-hints
tomcat-early-hints

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants