#78 ✓ resolved
raggi

Relax Lint slightly to allow subclasses of the required types

Reported by raggi | October 5th, 2009 @ 10:00 PM | in 1.1

In order to better service XSS protections, rails-core desires to be able to pass subclasses of string in the body portion of the response tuple.

Passing subclasses for IO bound operations seems reasonable, as does subclassing the types given in the spec. The given patch relaxes all instance_of? predicates to kind_of?.

If anyone has a good reason to say "no", please weigh in now.

Comments and changes to this ticket

  • Michael Koziarski

    Michael Koziarski October 5th, 2009 @ 10:10 PM

    Just a +1 from me, I've asked the same question on the list (without seeing this message).

    An alternative would be requiring them to respond to .to_s which would be slightly more flexible (allowing delayed operations which aren't string subclasses)

  • raggi

    raggi October 5th, 2009 @ 10:45 PM

    Yeah, Pratik pinged me on IRC about it, but I didn't catch your mail on the ML.

    Wrt. using #to_s, I'm not sure that'll generally be necessary, and would also mean that we'd be adjusting the spec for handlers. The general idea is that the value be something that could be safely passed to IO directly.

    It's possible to mess with #each to produce delayed operations, as I've done in async_sinatra and the async patches to Thin, when using DeferrableBody.

  • ericwong

    ericwong October 5th, 2009 @ 11:27 PM

    As I noted on the mailing list, this will break some C extensions (and maybe
    Java ones too). The Hash relaxation might break more than the String ones.
    MRI already does rb_obj_as_string() on all objects passed for writing to
    IO objects so there's no need for pure-Ruby server handlers to change and
    use #to_s.

  • Michael Koziarski

    Michael Koziarski October 6th, 2009 @ 12:32 AM

    I don't really have a horse in the race for the Hash cases, but the string case seems unlikely to break anything.

    Can you give an example of what might break, perhaps we can test it?

  • Joshua Peek

    Joshua Peek October 6th, 2009 @ 02:37 AM

    • State changed from “new” to “open”

    +1

    I would like to get the to_s spec passed, but the patch raggi posted will solve our Rails issue.

    Anyway this can go in 1.0.1? If it needs to go in 1.1, we should release 1.1 soon.

  • raggi

    raggi October 6th, 2009 @ 08:00 AM

    I would say kind_of may be suitable for 1.0.1, but possibly to_s would be a 1.1 level change.

    Wondering if any others of core have input?

  • raggi

    raggi October 6th, 2009 @ 02:14 PM

    In master now.

    Should end up in the 1.0.1 branch when that's created.

  • raggi

    raggi October 6th, 2009 @ 02:15 PM

    • Milestone cleared.

    That being said, it is a spec change, but a minor one... up to others.

    Leaving the ticket open to indicate requirement for merge into a release branch.

  • raggi

    raggi October 6th, 2009 @ 02:24 PM

    • State changed from “open” to “resolved”
  • raggi

    raggi October 6th, 2009 @ 02:36 PM

    • Milestone set to 1.1

    Ok, spoke to chris2, this should be a 1.1 change (for kind_of). Chris and I agree that the to_s approach is probably excessive after loosening of the spec for subclasses. Furhter specialisation can be done through the body container if necessary.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Attachments

Pages