Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: Fix segfault
Ooooh interesting, I see (makes a lot of sense)! I can confirm, btw; your updated fix (from 324509) works for me (no segfault + clean shutdown)! ... I will comment on the other thread as soon as the email comes through! Thanks a lot! Armin On Wed, 27 Mar 2024 at 18:21, Andreas Rheinhardt < andreas.rheinha...@outlook.com> wrote: > Armin Hasitzka via ffmpeg-devel: > > Hi, > > > > we found this when testing with rejected RTMP streams. > > > > Best > > Armin > > Thanks for the report and the patch; yet actually freeing them manually > is not only harmful, but also unnecessary, as these strings can be set > via AVOptions; therefore they will be freed generically (which is also > the reason why rtmp_close() does not have code to free them). I sent a > patch for this here: > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-March/324509.html Would > be nice if you could test it. > > - Andreas > > ___ > 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". > ___ 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".
[FFmpeg-devel] [PATCH] avformat/rtmpproto: Fix segfault
Hi, we found this when testing with rejected RTMP streams. Best Armin 0001-avformat-rtmpproto-Fix-segfault.patch Description: Binary data ___ 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".
Re: [FFmpeg-devel] [PATCH] avformat/libsrt: Decode URL parameter strings
I think accepting URL params in first place isn't inherently bad (depending
on the use case, it might make certain interactions easier).
I expected regressions to be brought up and just want to point out that,
while absolutely possible, actual regressions will likely be fairly rare:
- any "unencoded" characters will continue to work (eg "/" passed in
will continue to be "/")
- usage of "+" and "&" will cause unintended behaviour already ("="
_might_ cause unintended behaviour today)
- the main regression comes from "%.." substrings that are _not_ meant
to be url URL encoded but happen to be that and will now be turned into
something else
Because of this, I did not expect this patch to make it into any patch or
minor release. We were hoping though, that this would be a contender for
the next major one.
On Fri, 11 Aug 2023 at 03:55, "zhilizhao(赵志立)"
wrote:
>
>
> > On Aug 11, 2023, at 00:13, Armin Hasitzka 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(>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(>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(>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".
>
___
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".
[FFmpeg-devel] [PATCH] avformat/libsrt: Decode URL parameter strings
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 :) Best Armin 0001-avformat-libsrt-Decode-URL-parameter-strings.patch Description: Binary data ___ 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".
Re: [FFmpeg-devel] [PATCH] avformat/hls: Forward stream metadata from subdemuxer
Hi all, just checking; do you consider this for merge into the upstream? It does feel like a lot of upside without any downside. Thanks Armin On Tue, 23 May 2023 at 11:26, Armin Hasitzka wrote: > We found this when we were looking for language information that the > mpegts demuxer knew about, but the HLS demuxer didn't (get forwarded). > > Best > Armin > ___ 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".
[FFmpeg-devel] [PATCH] avformat/hls: Forward stream metadata from subdemuxer
We found this when we were looking for language information that the mpegts demuxer knew about, but the HLS demuxer didn't (get forwarded). Best Armin 0001-avformat-hls-Forward-stream-metadata-from-subdemuxer.patch Description: Binary data ___ 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".
Re: [FFmpeg-devel] Location header to be ignored with status code 200 (and others)
Cool :) Please let me know if the attached patch works for you (tried to follow ffmpeg's code style). Armin On Tue, 1 Oct 2019 at 12:16, Carl Eugen Hoyos wrote: > Am Di., 1. Okt. 2019 um 10:32 Uhr schrieb Armin Hasitzka >: > > > sorry for dumping this here with HTML formatting (haven't yet figured out > > how to turn it off in Gmail); we ran into a weird HLS playlist today that > > sends a malformed location header together with a 200 status code (which > is > > a misconfiguration on a client's system but cannot be changed). A quick > dig > > shows that location headers are to be ignored in combination with > anything > > else but a 201 or 3xx ( > > https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location) > thus we > > patched our FFmpeg version as attached. > > > We further believe that this patch should be applied to FFmpeg for > > everyone - please let us know what you think. > > Please use git format-patch to produce a patch that can be committed > with git am. > > Carl Eugen > ___ > 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". -- *Armin Hasitzka* Software Engineer t: +44 (0) 20 3745 6566 <https://twitter.com/Grabyo> <https://www.facebook.com/grabyo/> <https://www.linkedin.com/company/grabyo/> Check out our knowledge base for additional support <https://help.grabyo.com/hc/en-us> ffmpeg.http.location.2.patch Description: Binary data ___ 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".
[FFmpeg-devel] Location header to be ignored with status code 200 (and others)
Hi guys, sorry for dumping this here with HTML formatting (haven't yet figured out how to turn it off in Gmail); we ran into a weird HLS playlist today that sends a malformed location header together with a 200 status code (which is a misconfiguration on a client's system but cannot be changed). A quick dig shows that location headers are to be ignored in combination with anything else but a 201 or 3xx ( https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location) thus we patched our FFmpeg version as attached. We further believe that this patch should be applied to FFmpeg for everyone - please let us know what you think. Thanks Armin -- *Armin Hasitzka* Software Engineer t: +44 (0) 20 3745 6566 <https://twitter.com/Grabyo> <https://www.facebook.com/grabyo/> <https://www.linkedin.com/company/grabyo/> Check out our knowledge base for additional support <https://help.grabyo.com/hc/en-us> ffmpeg.http.location.patch Description: Binary data ___ 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".
