On 12 October 2012 20:24, Oleg Kalnichevski <[email protected]> wrote: > On Fri, 2012-10-12 at 03:09 +0100, sebb wrote: >> On 11 October 2012 15:47, Oleg Kalnichevski <[email protected]> wrote: >> > On Wed, 2012-10-10 at 22:11 +0100, sebb wrote: >> >> On 10 October 2012 21:51, <[email protected]> wrote: >> >> > Author: olegk >> >> > Date: Wed Oct 10 20:51:30 2012 >> >> > New Revision: 1396793 >> >> > >> >> > URL: http://svn.apache.org/viewvc?rev=1396793&view=rev >> >> > Log: >> >> > HTTPCLIENT-1248: Default and lax redirect strategies should not convert >> >> > requests redirected with 307 status to GET method >> >> > >> > >> > ... >> > >> >> The above code converts unknown methods into GET. >> >> If a new method is added, that will be incorrect. >> >> >> >> It might be useful to know when this is happening; maybe an assert >> >> would be useful here? >> >> Or a log message? >> >> >> >> Or is it possible to write a clone/copy method that works for all >> >> possible methods? >> >> Maybe methods should know how to clone themselves? >> >> >> > >> > This is a good point. I reworked the method to make use of the new >> > RequestBuilder class to create a mutable copy of the original request >> > prior to setting the request URI. >> > >> > http://svn.apache.org/viewvc?rev=1397085&view=rev >> > >> > Please review >> >> Looks much better and less fragile. >> >> Not quite sure why getMethod should default to POST/GET if the method >> is null - why should the method ever be null? Isn't that a bug? >> > > I think these are most commonly used methods and as such are reasonable > defaults if someone explicitly sets request method to null. I believe it > is should be ok to default to POSt for entity closing requests and GET > otherwise. >
Well, yes, if a null method is allowed then the defaults are fine. But I question whether one should allow a null method in the first place. Seems to me that could hide bugs, and is a bit confusing unless the default is universally applied. I think it would be safer to throw NPE or IAE. >> Also, method names are case-significant, so should probably not upcase. >> > > Good catch. Corrected. Thanks - it was only recently I saw the comment on the Tomcat list. > Oleg > >> Per the Tomcat issues list: >> >> >> >> RFCs 2616, 2068, and 1945 all agree that method name is case-sensitive: >> http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.1 >> << >> > Oleg >> > >> > >> > --------------------------------------------------------------------- >> > To unsubscribe, e-mail: [email protected] >> > For additional commands, e-mail: [email protected] >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
