mauricioaniche / restfulie.net

.net version of restfulie server
http://restfulie.caelum.com.br
Other
50 stars 17 forks source link

Certain accept headers crash restfulie #12

Closed dotnetchris closed 12 years ago

dotnetchris commented 12 years ago

Accept: text/html,application/xhtml+xml,application/xml;image/png,image/jpeg,image/;q=0.9,/*;q=0.8

This will crash

AcceptHeaderToMediaType private FormatPlusQualifier ParseFormat(string type)

 if (ContainsQualifier(type))
            {
                var typeInfo = type.Split(';');
                format = typeInfo[0].Trim();
                qualifier = Convert.ToDouble(typeInfo[1].Split('=')[1], new CultureInfo("en-US"));
            }

This gets an instance of "application/xml;image/png" which it then splits at the ; and then tries to resplit on =, however since there is no = at all, typeInfo[1] is out of bounds of the array.

System.IndexOutOfRangeExceptionIndex was outside the bounds of the array.

System.IndexOutOfRangeException: Index was outside the bounds of the array. at Restfulie.Server.Negotiation.AcceptHeaderToMediaType.ParseFormat(String type) at Restfulie.Server.Negotiation.AcceptHeaderToMediaType.GetMediaType(String acceptHeader) at Restfulie.Server.ActAsRestfulie.OnActionExecuting(ActionExecutingContext filterContext)

 [TestFixture]
    public class AcceptHeaderToMediaTypeTests
    {

        [Test]
        public void shouldwork()
        {
            var mediaType = acceptHeader.GetMediaType("text/html,application/xhtml+xml,application/xml;image/png,image/jpeg,image/*;q=0.9,*/*;q=0.8");
        }
dotnetchris commented 12 years ago

Actually, i see this bug was resolved in https://github.com/mauricioaniche/restfulie.net/commit/311764b3f867bd172ab1d327406a5908d694edb8 however the nuget package for this seems to be very out of date then.

mauricioaniche commented 12 years ago

uuh, shit! if you run your test in the latest version, does it work?

i am not sure who submitted the package to nuget! let me check!

pedroreys commented 12 years ago

I did, @mauricioaniche.

Just pushed the latest bits to nuget. Package version 0.5.4.

I'm going to talk with the codebetter folks to have restfulie on their CI server so that Teamcity publishes new packages automatically.

mauricioaniche commented 12 years ago

That would be awesome!

Chris, can we close the issue? On Jul 24, 2012 3:14 AM, "Pedro Reys" < reply@reply.github.com> wrote:

I did, @mauricioaniche.

Just pushed the latest bits to nuget. Package version 0.5.4.

I'm going to talk with the codebetter folks to have restfulie on their CI server so that Teamcity publishes new packages automatically.


Reply to this email directly or view it on GitHub:

https://github.com/mauricioaniche/restfulie.net/issues/12#issuecomment-7199504

dotnetchris commented 12 years ago

I guess, I can't verify whether this is good or not. I can't install any package that takes a hard dependency on Newtonsoft.Json > 4.0.8 because RavenDB actually needs a specific version.

Does restfulie TRULY need 4.5.6? Or does it only need > 4?

pedroreys commented 12 years ago

Bummer.

Let me do some testing but I believe it needs to be 4.5.6. Will spend some time on it tonight.

dotnetchris commented 12 years ago

@pedroreys thanks, i'll keep this monitored. At the very least I can backport the code to fix the header issue if I need to even if i can't update to the latest.

dotnetchris commented 12 years ago

Were you able to reach any conclusions to whether restfulie requires a hard dependency to that version of JSON.NET?

pedroreys commented 12 years ago

@dotnetchris nuget package 0.5.5.0 depends on JSON.NET 4.0.8 now. I also lowered the version requirements on Castle.Core to 2.5.2 to mitigate the risk of someone else having a similar issue.

Please let me know if it fixes your initial issue.

dotnetchris commented 12 years ago

Excellent. I just update-package'd and recompiled, turned on ModHeaders put that accepts header in chrome and went to my app and it loaded fine and served the correct razor view as it should have to the web browser.