> On Aug 11, 2023, at 00:13, Armin Hasitzka <ar...@grabyo.com> wrote: > > Hi again, > > we found this the other day when using a stream ID containing "%2F", > expecting this to be resolved to "/". While "%2F" could obviously be sent > decoded, "&" (decoded) would currently end the value and not be used, "+" > (decoded) would be overwritten with " ", and "=" (decoded) could cause > entirely unexpected outcomes (albeit all these characters being allowed by > SRT for various string inputs). > > As for changing `av_strndup` to `ff_urldecode` (which removes a length > check); I don't think that this particular length check added any > protection (`av_find_info_tag` adds a trailing `\0` if found). This > thinking is also evident at the two other places where `ff_urldecode` > replaced `av_strdup`. > > It would be amazing if we could get this merged upstream :) >
Thanks for the contribution, but the issue is kind of complex. 1. The format of streamid isn’t take URL into consideration "#!::u=admin,r=foo” 2. Obviously some implementation and usecases depend on these fields to be passed directly without URL encoding/decoding. The same issue has been filed to srt again and again, ref. https://github.com/Haivision/srt/issues/2749 and #1871, #1173, #2015. I think we shouldn’t put these fields into URL at the first place. Now doing any change, even if it’s technically correct, will make regression issues. I have no idea to improve the code. But it’s easy to solve from the user’s point of view: don’t set these fields via URL, just use options. These is no implementation dependent behavior with options. > --- > libavformat/libsrt.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c > index cd8f5b1e7d..8986616334 100644 > --- a/libavformat/libsrt.c > +++ b/libavformat/libsrt.c > @@ -32,6 +32,7 @@ > #include "network.h" > #include "os_support.h" > #include "url.h" > +#include "urldecode.h" > > /* This is for MPEG-TS and it's a default SRTO_PAYLOADSIZE for SRTT_LIVE (8 > TS packets) */ > #ifndef SRT_LIVE_DEFAULT_PAYLOAD_SIZE > @@ -547,7 +548,7 @@ static int libsrt_open(URLContext *h, const char *uri, > int flags) > } > if (av_find_info_tag(buf, sizeof(buf), "passphrase", p)) { > av_freep(&s->passphrase); > - s->passphrase = av_strndup(buf, strlen(buf)); > + s->passphrase = ff_urldecode(buf, 0); > } > #if SRT_VERSION_VALUE >= 0x010302 > if (av_find_info_tag(buf, sizeof(buf), "enforced_encryption", p)) { > @@ -632,7 +633,7 @@ static int libsrt_open(URLContext *h, const char *uri, > int flags) > } > if (av_find_info_tag(buf, sizeof(buf), "streamid", p)) { > av_freep(&s->streamid); > - s->streamid = av_strdup(buf); > + s->streamid = ff_urldecode(buf, 0); > if (!s->streamid) { > ret = AVERROR(ENOMEM); > goto err; > @@ -640,7 +641,7 @@ static int libsrt_open(URLContext *h, const char *uri, > int flags) > } > if (av_find_info_tag(buf, sizeof(buf), "smoother", p)) { > av_freep(&s->smoother); > - s->smoother = av_strdup(buf); > + s->smoother = ff_urldecode(buf, 0); > if(!s->smoother) { > ret = AVERROR(ENOMEM); > goto err; > -- > 2.41.0 > > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
