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.

Sign up for GitHub

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

Jump to bottom

[bug] Regression in 1.13.0 in XSLT.quote_params #2418

Closed
jcoyne opened this issue Jan 10, 2022 · 7 comments · Fixed by #2420
Closed

[bug] Regression in 1.13.0 in XSLT.quote_params #2418

jcoyne opened this issue Jan 10, 2022 · 7 comments · Fixed by #2420
Milestone
v1.13.x patch rel...

Comments

@jcoyne
Copy link

jcoyne commented Jan 10, 2022

In nokogiri 1.12 and before I was able to send integers to XSLT.quote_params:

Nokogiri::XSLT.quote_params(['foo', 123])

After upgrading to 1.13.0 this results in a TypeError:

*** TypeError Exception: no implicit conversion of Integer into String
@jcoyne jcoyne added the state/needs-triage Inbox for non-installation-related bug reports or help requests label Jan 10, 2022
jcoyne added a commit to sul-dlss/gis-robot-suite that referenced this issue Jan 10, 2022
@jcoyne
Nokogiri 1.13 does not permit integers as XSLT parameters
9bfb2f2 
See sparklemotion/nokogiri#2418
@flavorjones
Copy link
Member

@jcoyne Thanks for opening this! I'll take a look.

@flavorjones
Copy link
Member

git bisect isolated this regression to 3099528 which is ... 🤔

Should be an easy fix, though.

@flavorjones
Copy link
Member

Basically:

1234 =~ /'/ # this is OK
/'/.match?(1234) # this is not OK

@flavorjones flavorjones removed the state/needs-triage Inbox for non-installation-related bug reports or help requests label Jan 10, 2022
@flavorjones flavorjones added this to the v1.13.x patch releases milestone Jan 10, 2022
@CvX CvX mentioned this issue Jan 10, 2022
Closed
flavorjones added a commit that referenced this issue Jan 11, 2022
@flavorjones
fix: regression in XSLT.quote_params handling non-strings
7e84cd8 
Along the way, rewrite this method, refactor the tests, and improve
the documentation.

Closes #2418
@flavorjones flavorjones mentioned this issue Jan 11, 2022
Merged
@flavorjones
Copy link
Member

PR at #2420

flavorjones added a commit that referenced this issue Jan 11, 2022
@flavorjones
fix: regression in XSLT.quote_params handling non-strings
cc6ff71 
Along the way, rewrite this method, refactor the tests, and improve
the documentation.

Closes #2418
flavorjones added a commit that referenced this issue Jan 11, 2022
@flavorjones
fix: regression in XSLT.quote_params handling non-strings
ce240d8 
Along the way, rewrite this method, refactor the tests, and improve
the documentation.

Closes #2418
@flavorjones flavorjones mentioned this issue Jan 11, 2022
Merged
@jcoyne
Copy link
Author

jcoyne commented Jan 11, 2022

Thank you @flavorjones !

@flavorjones
Copy link
Member

I'm hoping to ship v1.13.1 today or tomorrow. Working on one other regression.

@flavorjones
Copy link
Member

v1.13.1 is out now: https://github.com/sparklemotion/nokogiri/releases/tag/v1.13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.13.x patch releases
Development

Successfully merging a pull request may close this issue.

fix: regression in XSLT.quote_params handling non-strings sparklemotion/nokogiri
2 participants
@flavorjones @jcoyne