On Wed, Aug 19, 2015 at 10:24 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > On Wed, Aug 19, 2015 at 9:14 PM, Stephan Holljes > <klaxa1...@googlemail.com> wrote: >> On Thu, Aug 20, 2015 at 2:39 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: >>> On Wed, Aug 19, 2015 at 7:59 PM, Stephan Holljes >>> <klaxa1...@googlemail.com> wrote: >>>> On Thu, Aug 20, 2015 at 12:11 AM, Ganesh Ajjanagadde <gajja...@mit.edu> >>>> wrote: >>>>> On Wed, Aug 19, 2015 at 12:14 PM, Stephan Holljes >>>>> <klaxa1...@googlemail.com> wrote: >>>>>> --- >>>>>> libavformat/http.c | 8 ++++++++ >>>>>> 1 file changed, 8 insertions(+) >>>>>> >>>>>> diff --git a/libavformat/http.c b/libavformat/http.c >>>>>> index a136918..4dbef3f 100644 >>>>>> --- a/libavformat/http.c >>>>>> +++ b/libavformat/http.c >>>>>> @@ -348,11 +348,19 @@ static int http_write_reply(URLContext* h, int >>>>>> status_code) >>>>>> reply_text = "OK"; >>>>>> content_type = "application/octet-stream"; >>>>>> break; >>>>>> + case 301: >>>>>> + reply_code = 301; >>>>>> + reply_text = "Moved Permanently"; >>>>>> + break; >>>>> >>>>> 301 is usually used for URL redirection, >>>>> and you don't seem to do anything beyond setting the message. >>>>> There needs to be additional logic somewhere to handle this. >>>>> Nevertheless, it is still ok as a patch to me, >>>>> since I assume this will be handled later on. >>>>> I strongly suggest adding something to clarify this in the commit message. >>>> >>>> I did not think about that and so far I have only used it in my >>>> modified ffserver code. What makes a 301 reply different from the >>>> other replies? Maybe I didn't understand the RFC correctly. >>> >>> Quoting from the RFC: >>> "The new permanent URI SHOULD be given by the Location field in the >>> response. >>> Unless the request method was HEAD, >>> the entity of the response SHOULD contain a short hypertext note with >>> a hyperlink to the new URI(s). " >>> >>> My point is that somewhere in the response a new URI must be provided, >>> so that the client can go to the redirected location. >>> I don't know where/how such logic should go, but it needs to be done. >>> >> >> Ah yes, thus far the application had to take care of that. Using the >> location field in HTTPContext seems like a good place to store this >> information. I will work on a patch that includes this. Thanks! > > BTW, are you sure line 383 is right? > message_len could be set above sizeof(message) (if we hit the buffer > size limit). > I do not think (but can't confirm easily) that ffurl_write requires > size < sizeof(buf).
sorry, typo: do not think -> think > See e.g line 1097 - use the strlen function to get the correct message length, > or alternatively (to avoid the strlen call) saturate the return value > of snprintf > to the value you need. > > In fact, I don't know why strlen is being used in 1097, > it is an additional call that can be avoided by utilizing return value > of the snprintf. > You might want to look into this and similar instances. > >> >>>> >>>>> >>>>>> case AVERROR_HTTP_SERVER_ERROR: >>>>>> case 500: >>>>>> reply_code = 500; >>>>>> reply_text = "Internal server error"; >>>>>> break; >>>>>> + case 503: >>>>>> + reply_code = 503; >>>>>> + reply_text = "Service Unavailable"; >>>>>> + break; >>>>> >>>>> Looks ok to me. >>>>> >>>>>> default: >>>>>> return AVERROR(EINVAL); >>>>>> } >>>>>> -- >>>>>> 2.1.0 >>>>>> >>>>>> _______________________________________________ >>>>>> ffmpeg-devel mailing list >>>>>> ffmpeg-devel@ffmpeg.org >>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>> _______________________________________________ >>>>> ffmpeg-devel mailing list >>>>> ffmpeg-devel@ffmpeg.org >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
