Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert CDATA input to string before gsub'ing #12693

Merged

Conversation

carpodaster
Copy link
Contributor

Rails 3.2 API allowed arbitrary input for cdata_section; this change re-introduces the old behaviour.

If it is deemed worthy to be merged, does it make sense to add it to 4.0-STABLE as well?

Rails 3.2 API allowed arbitrary input for cdata_section;
this change re-introduces the old behaviour.
@rafaelfranca
Copy link
Member

I really don't see the point to pass non string values to this method. The documentation only shows example with string values.

Could you explain why you want to pass arbitrary values?

@carpodaster
Copy link
Contributor Author

I'll gladly explain it.

My use case is a Rails 3.2 app that uses cdata_section for building XML output. The method is called several times and receives values of ActiveModel instance attributes. These attributes are oftentime nil or DateTime instances.

When you pass data that is potentially nil to cdata_section, it will break due to the unchecked #gsub. Yes, you can add a guarding if for every call to cdata_section but to me it seems reasonable to pull that piece of input sanitation into the method being called.

DateTime objects have a pretty reasonable #to_s. No need to bow out with a undefined method error there, either.

Furthermore, there is no damage done: When you call cdata_section you'll expect to get a CDATA string. Worst thing that could happen is that this tag will be empty. The ideal case is that it Just Works(tm).

We can and should be lazy about the type being passed in.

@rafaelfranca
Copy link
Member

I see. Thank you for the explanation. This help to take the decision to backport or not. Agree with the fix. Thanks

rafaelfranca added a commit that referenced this pull request Oct 29, 2013
Convert CDATA input to string before gsub'ing
@rafaelfranca rafaelfranca merged commit 671af07 into rails:master Oct 29, 2013
rafaelfranca added a commit that referenced this pull request Oct 29, 2013
Convert CDATA input to string before gsub'ing
@carpodaster carpodaster deleted the convert-cdata-input-to-string branch October 29, 2013 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants