-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Fix RTSP basic auth #9870
Fix RTSP basic auth #9870
Conversation
Verified with test 3100 Fixes #4750 Closes #....
Once more a comment on closed PR... Seems to make sense though |
Agreed. It's even strange that it can update the first host... |
Suggested-by: Erik Janssen URL: #9870 (comment)
Suggested-by: Erik Janssen URL: #9870 (comment) Closes #9882
* safe assumption as no other redirects should happen from RTSP. | ||
*/ | ||
if(conn->handler->protocol & CURLPROTO_RTSP) | ||
data->set.redir_protocols = CURLPROTO_RTSP; |
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 probably breaks reuse of handle with another protocol.
If handle is used for RTSP then for HTTP, redirection won't work on HTTP unless reset externally between both uses.
I would rather use a copy of set.redir_protocols
into a new state.redir_protocols
field in the whole library.
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.
Yikes, that is indeed a good remark. I need to fix this...
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.
The RTSP code also wrongly sets data->set.opt_no_body
in several places...
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 did not see ithem before but you're right. I noticed this particular one while rebasing the sieve PR, as it also features redirection.
I've just done a quick check for more and there may be some (although not all listed):
$ find lib -name '*.c' | xargs grep -e '->set\.[A-Za-z0-9_.]* *= ' | grep -Fv 'setopt.c
> url.c'
lib/easy.c: dst->set.postfields = dst->set.str[i];
lib/easy.c: outcurl->set.buffer_size = data->set.buffer_size;
lib/http.c: data->set.redir_protocols = CURLPROTO_RTSP;
lib/ftp.c: data->set.ftp_filemethod = FTPFILE_MULTICWD;
lib/ftp.c: data->set.fwrite_func = Curl_ftp_parselist;
lib/ftp.c: data->set.out = data;
lib/ftp.c: data->set.fwrite_func = ftpwc->backup.write_function;
lib/ftp.c: data->set.out = ftpwc->backup.file_descriptor;
lib/doh.c: doh->set.fmultidone = doh_done;
lib/doh.c: doh->set.dohfor = data; /* identify for which transfer this is done */
lib/rtsp.c: data->set.opt_no_body = TRUE; /* most requests don't contain a body */
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = TRUE;
lib/transfer.c: data->set.trailer_data = NULL;
lib/transfer.c: data->set.trailer_callback = NULL;
lib/transfer.c: data->set.upload = false;
lib/http2.c: node->data->set.stream_depends_on = child;
lib/http2.c: parent->set.stream_dependents = 0;
lib/http2.c: (*tail)->data->set.stream_depends_e = FALSE;
lib/http2.c: child->set.stream_depends_on = parent;
lib/http2.c: child->set.stream_depends_e = exclusive;
lib/http2.c: parent->set.stream_dependents = data->next;
lib/http2.c: child->set.stream_depends_on = 0;
lib/http2.c: child->set.stream_depends_e = FALSE;
lib/multi.c: data->state.conn_cache->closure_handle->set.timeout = data->set.timeout;
lib/conncache.c: connc->closure_handle->set.buffer_size = READBUFFER_MIN;
lib/vtls/vtls.c: data->set.general_ssl.max_ssl_sessions = amount;
$
Maybe we can plan to have only setopt values in set
and r/w runtime values in state
? Just an idea...
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.
Maybe we can plan to have only setopt values in set and r/w runtime values in state?
I think we should, yes. And then possibly invent and set up some some kind of framework/markup or something that lets us scan for mistakes like the present.
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.
#9888 addresses parts of this
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.
And then possibly invent and set up some some kind of framework/markup
Suggestion:
- declare
set
asconst
- use a cast with a comment in the few functions allowed to alter it.
Not very elegant but will cause a compile error if such a mistake occurs again. The C language does not offer many alternatives to achieve that. And it does not require any external test or tool.
known bug 6.8