You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2024/11/13 23:21:09 UTC
[PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
bneradt opened a new pull request, #11859:
URL: https://github.com/apache/trafficserver/pull/11859
This is a very similar change as #11789. With this patch, TransformTerminus properly supports a 0 byte read to cancel VIO. Without this patch, the logic naively scheduled an event which called the handler which would crash on what was often a canceled continuation or otherwise invalid continuation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #11859:
URL: https://github.com/apache/trafficserver/pull/11859#issuecomment-2536580347
[approve ci autest freebsd]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #11859:
URL: https://github.com/apache/trafficserver/pull/11859
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #11859:
URL: https://github.com/apache/trafficserver/pull/11859#discussion_r1884176781
##########
include/proxy/TransformInternal.h:
##########
@@ -49,6 +50,12 @@ class TransformTerminus : public VConnection
int m_deletable;
int m_closed;
int m_called_user;
+
+private:
+ Event *_read_event = nullptr;
+ bool _read_disabled = false;
+ Event *_write_event = nullptr;
+ bool _write_disabled = false;
Review Comment:
That's a good point. My logic was to default this new "feature" I'm adding to off.
The initial values shouldn't make a difference, since the events we're protecting only happen during the do_io_* function calls which will explicitly set these to true or false, but I see your logic for defaulting the flags to disabled since the buffer will be nullptr. I'll run a few quick internal tests in production to verify I'm not missing something and update the PR if it seems fine.
Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #11859:
URL: https://github.com/apache/trafficserver/pull/11859#discussion_r1884215910
##########
include/proxy/TransformInternal.h:
##########
@@ -49,6 +50,12 @@ class TransformTerminus : public VConnection
int m_deletable;
int m_closed;
int m_called_user;
+
+private:
+ Event *_read_event = nullptr;
+ bool _read_disabled = false;
+ Event *_write_event = nullptr;
+ bool _write_disabled = false;
Review Comment:
Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TransformTerminus::do_io_*: handle null buffer [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11859:
URL: https://github.com/apache/trafficserver/pull/11859#discussion_r1882508257
##########
include/proxy/TransformInternal.h:
##########
@@ -49,6 +50,12 @@ class TransformTerminus : public VConnection
int m_deletable;
int m_closed;
int m_called_user;
+
+private:
+ Event *_read_event = nullptr;
+ bool _read_disabled = false;
+ Event *_write_event = nullptr;
+ bool _write_disabled = false;
Review Comment:
Is it safer for the disabled flags to default to true?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org