Rails core - Nov 2006 - Changeset 5634/5635 Break Integration Tests (with follow_redirect!)

Changing the redirect header to use ''Location'' rather than
''location''
breaks integration tests that use follow_redirect. Look at
integration.rb:279 (parse_result):

        def parse_result
          headers, result_body = @result.split(/\r\n\r\n/, 2)

          @headers = Hash.new { |h,k| h[k] = [] }
          headers.each_line do |line|
            key, value = line.strip.split(/:\s*/, 2)
            @headers[key.downcase] << value
          end

For some reason header keys as returned from the result are all
downcased, which means that ''location'' rather than
''Location'' is set
in the response header. So, now ''Location'' is nil and I get
URI parse
errors.

I PDId, how should we fix this?

-- 
Kevin Clark
http://glu.ttono.us

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Changing the redirect header to use ''Location'' rather
than ''location''
> breaks integration tests that use follow_redirect. Look at
> integration.rb:279 (parse_result):
Well, the header isn''t the problem it''s the change in
integration.rb.
Normalising http headers is reasonably common, I believe the servlet
API does it too (enterprise!).

If changing that line back to location fixes it,  then we can just commit that.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Mon, November 27, 2006 2:25 pm, Kevin Clark wrote:
> Changing the redirect header to use ''Location'' rather
than ''location''
> breaks integration tests that use follow_redirect. Look at
> integration.rb:279 (parse_result):
>
>         def parse_result
>           headers, result_body = @result.split(/\r\n\r\n/, 2)
>
>           @headers = Hash.new { |h,k| h[k] = [] }
>           headers.each_line do |line|
>             key, value = line.strip.split(/:\s*/, 2)
>             @headers[key.downcase] << value
>           end
>
> For some reason header keys as returned from the result are all
> downcased, which means that ''location'' rather than
''Location'' is set
> in the response header. So, now ''Location'' is nil and I
get URI parse
> errors.
>
> I PDId, how should we fix this?
>
> --
> Kevin Clark
> http://glu.ttono.us
I think this change should be backed out. Especially since the
ActionController tests don''t pass.

At this point dealing with the cascade effects of getting rid of the
downcase is going to be pretty messy. This isn''t the sort of change you
want to be making in a release candidate.

-- 
Josh Susser
http://blog.hasmanythrough.com


--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

I agree the problem isn''t in parse_result, the problem is in the
changeset (which changed the header to Location instead of location).
Either way, if the AP tests pass when the change is made things should
work, one way or another.

Kev

On 11/27/06, Michael Koziarski <michael@koziarski.com>
wrote:
>
> > Changing the redirect header to use ''Location''
rather than ''location''
> > breaks integration tests that use follow_redirect. Look at
> > integration.rb:279 (parse_result):
>
> Well, the header isn''t the problem it''s the change in
integration.rb.
> Normalising http headers is reasonably common, I believe the servlet
> API does it too (enterprise!).
>
> If changing that line back to location fixes it,  then we can just commit
that.
>
> --
> Cheers
>
> Koz
>
> >
>

-- 
Kevin Clark
http://glu.ttono.us

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> I think this change should be backed out. Especially since the
> ActionController tests don''t pass.
>
> At this point dealing with the cascade effects of getting rid of the
> downcase is going to be pretty messy. This isn''t the sort of
change you
> want to be making in a release candidate.
What went before it was worse. We''d set "Status" but
"location".
Getting one style for headers is much easier to deal with when you
reason about what should be done to the lot of them. So let''s fix the
tests and any trouble arising from this.


--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 11/28/06, Kevin Clark <kevin.clark@gmail.com>
wrote:
>
> I agree the problem isn''t in parse_result, the problem is in the
> changeset (which changed the header to Location instead of location).
> Either way, if the AP tests pass when the change is made things should
> work, one way or another.
The header should be Location, and follow_redirect should work with
the normalised headers.  [5641] takes care of this.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---