-
Notifications
You must be signed in to change notification settings - Fork 780
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
TS-5006: Fix CID 1356975 #1140
TS-5006: Fix CID 1356975 #1140
Conversation
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1083/ for details. |
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/976/ for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a Jira number.
@@ -3309,7 +3309,7 @@ HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c) | |||
|
|||
// only external POSTs should be subject to this logic; ruling out internal POSTs here | |||
bool is_eligible_post_request = (t_state.method == HTTP_WKSIDX_POST); | |||
if (is_eligible_post_request) { | |||
if (is_eligible_post_request && ua_session) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know this code much, but seeing that we don't crash here, presumably ua_session generally is not NULL here. So maybe a better thing is a release assert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @shinrich is looking at code further up where we check for NULLness of ua_session
so maybe she can say more whether or not we should be asserting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, looked at that. I don't think the NULL check around abort is necessary at all. So perhaps the easier thing is to remove that check.
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1093/ for details. |
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/985/ for details. |
ua_session can not be NULL so checking for it here only confuses coverity.
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1100/ for details. |
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/993/ for details. |
Check for NULL before pointer dereference.