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

Do not allow closing of the live input stream #1346

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

Conversation

laeubi
Copy link

@laeubi laeubi commented Oct 29, 2024

Currently the raw stream is passed to downstream consumers of RqLive, this has the problem that such consumers can close the stream. In case of a socket, this means the socket itself is closed and no response can be send by takes.

This wraps the original live input stream into one that simply ignores the close request.

Currently the raw stream is passed to downstream consumers of RqLive,
this has the problem that such consumers can close the stream. In case
of a socket, this means the socket itself is closed and no response can
be send by takes.

This wraps the original live input stream into one that simply ignores
the close request.
@laeubi
Copy link
Author

laeubi commented Oct 29, 2024

Another option would be to handle it on all different places where such streams are used, but it seems more clean to handle it more like at the roots.

@laeubi
Copy link
Author

laeubi commented Oct 29, 2024

One problem is that currently RqTemp uses RqLive in a way that relies on close... so one can also handle this in BkBasic

@yegor256 any guidance on this how it is best to be handled? Maybe have (another) wrapper RqNoClose ?!?

@laeubi
Copy link
Author

laeubi commented Oct 29, 2024

Testcase is currently also missing. Where would be the best place to place such test that fails if no response is given?

@yegor256
Copy link
Owner

@laeubi who is the consumer of the InputStream that may close it "accidentally"? Can you give an example?

@laeubi
Copy link
Author

laeubi commented Oct 29, 2024

In my case I have a POST handler with a TkRegex, there I read the body of the message with a function that closes the passed in stream. While I could workaround this there, it seems consumers should not be able to close the stream/socket as otherwise sending a response is no longer possible.

@yegor256
Copy link
Owner

@laeubi good point, I believe, you're right. However, I would implement it slightly differently. In the BkBasic I would replace this line:

new RsPrint(this.take.act(req)).print(output);

With this one:

new RsPrint(this.take.act(new RqUncloseable(req))).print(output);

It seems that this is what you suggested earlier as an alternative.

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.

2 participants