I've re-factored the code for this server, removing unnecessary lines and dispatching all work to the base services.module

Note: We should try to get this on the main services project. It's too useful to have to download an extra module, and it should get some "love" from the main project.

Comments

magico’s picture

Title: JSON server code refactored » Code refactored (support for GET and crossdomain requests)
StatusFileSize
new1.87 KB

Ok!

After those last changes I found that I needed crossdomain calls with the jQuery getJSON.
So I've implement GET requests and the respective "jsoncallback" used by jQuery.

I don't have a generic javascript code for this because, I want to refactor the server code, and think in a easy way to distribute a separate file to use. Perhaps just put the code in the Services handbook.

andremolnar’s picture

Status: Active » Needs work

I'd really like to make json_server work with GET as well. I think the patch needs work.

First off it looks like the required argument checking is gone.

I'd also like to have more thorough documentation of the usage of callback. Basically the introduction of 'callback' is an extra parameter that won't be defined by the individual services so people won't know to include it when making the request.

The biggest problem, now that I think about it, is that it doesn't look like you are doing any sort of validation on the callback argument.

services/json?callback=sinisterJavascript = bad news I think

andre

andremolnar’s picture

p.s. can you supply a proper patch next time - it makes things easier to review.

magico’s picture

StatusFileSize
new5.33 KB

You are right. I am in the middle of a complete rewrite of services to support many things (including multiple protocol and making this server a first class citizen).

1. Support for GET requests. Yes, but because there is a problem with use GET through crossdomain, my advice is that the support for GET should be always done using callback system. No need to have duplicated GET methods.

2. For this we need a nice .js file that we can use in any site (with services implemented or not). I attached a sample file that calls the json_server. From this file we need to take the "service" part that would be putted in drupal.js of all sites. BTW, remember that I am reworking services.module in a way that it would be used as the base of all services for Drupal 8 (this is my idea) and json_server would be used for all ajax calls.

3. json_server don't need to validate callback function arguments. services.module should do all the arguments validation and send errors/warnings.

I will have some news around next two weeks, but I made this to get some services working at my server and put it here just to let you guys know about it. If you like my strategy, let's continue working on this...

andremolnar’s picture

With point 3 - 'services.module should do all the arguments validation...'

True enough, but in the case of 'callback' - and especially in the proposed patch neither is doing the work for checking the input.

There are three options

  1. either each individual service/method decides whether or not it accepts a 'callback' argument and services handles validation/checking.
  2. here's a crazy thought: create a hook_services_alter that allows people to extend existing methods
  3. We bake in a 'callback' argument into json_server and handle the validation. (as your patch suggested)

The first one has problems because it would require all services/methods to be updated to accept 'callback' as an argument.
The second one requires changes to the services api.
The last one is a bit unexpected as it breaks out of the services api

andre

magico’s picture

I've correct my post above. By "callback" do you mean "jsoncallback"?

If you pass any javascript to it, the server will return it again. This does not make any harm to the server, and the only who will be affected is the caller. What kind of validation would be needed?

andremolnar’s picture

I've followed up via your contact form.

Yes I meant json_callback which gets renamed to $callback in the patch.

No there isn't much a security issue on the server side.

The security issue is with the client that makes the call to the server. See my email for the details of how that is a security issue.

drumm’s picture

StatusFileSize
new2.28 KB

A proper patch file from update #1 is attached.

scottgifford’s picture

This patch looks quite useful, we have a few similar changes in our local copy of json_services and it would be nice to get this into the main module. The status is "needs work", are there issues with this patch that haven't yet been addressed?

yajnin’s picture

Is there still love to see if we can have this module adopted by the Services core? I thought it was abandoned until I found this thread.

I need GET support so I'm going to try this. I twigged on the comment #1 regarding the need for jQuery. Is jQuery a dependency for this patch?

skyredwang’s picture

Status: Needs work » Closed (won't fix)