Feature #13718

Accept dots in JSONP callback

Added by Vitaliy Pitvalo over 5 years ago. Updated over 2 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:REST API
Target version:3.3.0
Resolution:Fixed

Description

When i try use mootools Request.JSONP Request it's not working.
For example http://avtehnik.m.redmine.org/projects.json?callback=my.Handler&key=0e96523a813ecf559ddba696a4f9549489aa8781 , the callback is my.Handler but in response dot is not appear myHandler({"projects":....

issue-13718.diff Magnifier (1.62 KB) Toshi MARUYAMA, 2015-12-21 05:44


Related issues

Related to Redmine - Feature #11469: JSONP support Closed

Associated revisions

Revision 15066
Added by Jean-Philippe Lang over 2 years ago

Accept dots in JSONP callback (#13718).

History

#1 Updated by Etienne Massip over 5 years ago

  • Tracker changed from Defect to Feature
  • Subject changed from Dots in JSONP Callback handler to Extend JSONP callback handler syntax support
  • Target version set to Candidate for next major release

According to json-p.org The proposed solution, allowed syntax could include these forms:

functionName({JSON});

obj.functionName({JSON});

obj["function-name"]({JSON});

For now, only the first one is partly supported whith functionName bein composed of digits and characters A to Z plus _ (see source:/trunk/lib/redmine/views/builders/json.rb@11272#L30).

Theoretically functionName should also be checked as being a valid javascript identifier name, thus allowing some Unicode characters, but this might look a bit overkill?

#2 Updated by Ken Franqueiro over 4 years ago

I would definitely agree that at least foo.bar(...) support should be added (which I assume is what this issue originally asked for specifically, based on the subject history).

Some JS libraries e.g. Dojo use a global namespace for their JSONP request support so that only one global variable is created instead of multiple, and currently the regex in http://www.redmine.org/projects/redmine/repository/entry/trunk/lib/redmine/views/builders/json.rb will strip periods, making Redmine's JSONP REST API unusable with these JS libraries.

Here's an example of the kind of function names Dojo's dojo/request/script module uses: dojo_request_script_callbacks.dojo_request_script0

#3 Updated by Sam Blowes almost 4 years ago

Angular JS also uses object based JSONP callbacks.

Please support this ! :)

https://github.com/angular/angular.js/issues/1551

#4 Updated by Sam Blowes almost 4 years ago

I added the dot to the regex, which at least fixes the obj.functionName({JSON}); example.

https://github.com/blowsie/redmine/commit/6d476160cdbc4eee17ae481f873fc07dc8cdf571

#5 Updated by Brian Bouterse about 3 years ago

I just spent several hours implementing workarounds because Redmine does not adhere to the JSONP specification. I'm still not done... I'm using Angular JS like the user in comment #3. This issue is years old and has a link to a commit (comment #4) that contains a 1 line fix.

What needs to happen for this to be included in the next Redmine release.

#6 Updated by Toshi MARUYAMA about 3 years ago

Sam Blowes wrote:

I added the dot to the regex, which at least fixes the obj.functionName({JSON}); example.

https://github.com/blowsie/redmine/commit/6d476160cdbc4eee17ae481f873fc07dc8cdf571

lib/redmine/views/builders/json.rb for trunk r13339:

@@ -27,7 +27,7 @@ def initialize(request, response)
           super
           callback = request.params[:callback] || request.params[:jsonp]
           if callback && Setting.jsonp_enabled?
-            self.jsonp = callback.to_s.gsub(/[^a-zA-Z0-9_]/, '')
+            self.jsonp = callback.to_s.gsub(/[^a-zA-Z0-9_.]/, '')
           end
         end

#7 Updated by Sam Blowes over 2 years ago

Could we pretty please have this single character added to this line?

Just for your sanity here is a description of the regex;

// [^a-zA-Z0-9_.]
// 
// Options: Case sensitive; ^$ match at line breaks; dot doesn’t match line breaks; Regex syntax only
// 
// Match any single character that is NOT present in the list below and that is NOT a line break character (line feed) «[^a-zA-Z0-9_.]»
//    A character in the range between “a” and “z” (case sensitive) «a-z»
//    A character in the range between “A” and “Z” (case sensitive) «A-Z»
//    A character in the range between “0” and “9” «0-9»
//    A single character from the list “_.” «_.»

#8 Updated by Sam Blowes over 2 years ago

Perhaps someone with SVN skills would kindly submit my patch to the Redmine SVN repo.
https://github.com/redmine/redmine/pull/68

Regards.

#10 Updated by Toshi MARUYAMA over 2 years ago

  • File issue-13718.diffMagnifier added
  • Target version changed from Candidate for next major release to 3.3.0

Sam Blowes wrote:

https://bitbucket.org/redmine/redmine/pull-requests/3/improve-jsonp-support/diff

One of reasons which pull request is bad is if source repository is deleted, diff is also deleted.

#11 Updated by Jean-Philippe Lang over 2 years ago

  • Subject changed from Extend JSONP callback handler syntax support to Accept dots in JSONP callback
  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

Fix committed, thanks.

Also available in: Atom PDF