electricimp / rocky Goto Github PK

2015-09-29 12:48:15 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed9d5a74e0)
2015-09-29 12:48:15 UTC+2   [Agent] [rocky] /commands/execute
2015-09-29 12:48:17 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed94184ff0)
2015-09-29 12:48:17 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed94184ff0)
2015-09-29 12:48:17 UTC+2   [Agent] [rocky] /commands/execute
2015-09-29 12:48:17 UTC+2   [Device]    [bull] commands/execute
2015-09-29 12:48:18 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed8de7d530)
2015-09-29 12:48:18 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed8de7d530)
2015-09-29 12:48:18 UTC+2   [Agent] [rocky] Request for commands/execute/, data: (array : 0x7fed8de7d530)
2015-09-29 12:48:18 UTC+2   [Agent] [rocky] /commands/execute
2015-09-29 12:48:18 UTC+2   [Device]    [bull] commands/execute

The lines with [rocky] Request are logged from the middleware handler:

app.use(function(context, next) {
  local path = ""

  foreach(subpath in context.path) {
    path += subpath
    path += "/"
  }

  server.log("[rocky] Request for " + path)

  next()
})

As suggested by the log lines, the middleware handler is called +1 for every new request.

Instead of hard coding access control for POST, PUT, GET and OPTIONS we should have an array of allowed methods which defaults to these values. Developers could then add extra headers like DELETE and PATCH to the list.

Scenario:

local contentType = "multipart/form-data; boundary=--ff4ed67396bc8e1d6dbf19d65b6c6348";
local body = "--ff4ed67396bc8e1d6dbf19d65b6c6348\r\nContent-Disposition: form-data; name=\"contentType\"\r\n\r\nbody\r\n--ff4ed67396bc8e1d6dbf19d65b6c6348\r\nContent-Disposition: form-data; name=\"tmp\"\r\n\r\nContent file\r\n--ff4ed67396bc8e1d6dbf19d65b6c6348";
local headers = {
    "content-type": contentType,
    "content-length": body.len()
};
local app = Rocky();
app.on("GET", "/test", function(context) {
    if ("table" != type(context.req.body)) {
        server.log("Wrong type of context.req.body: " + type(context.req.body) + ", should be table");
    }
    context.send(200, {"message": "OK"});
}.bindenv(this));
imp.wakeup(0, function() {
    local req = http.request("GET", http.agenturl() + "/test", headers, body);
    req.sendasync(function(res){});
}.bindenv(this));

Log:

[Agent] Wrong type of context.req.body: array, should be table

Tests:

This issue affects class: tests/CoreRockyMethod.nut method: testContentTypeMultipart()

Currently, we support the * wildcard for the verb in .on() requests to mean "any verb". We should consider supporting the same format for the path signature. This leads to questions of using wildcards instead of regular expressions for other parts of the signature such as /path/* instead of /path/.* but I am not sure if that is the right thing to do.

Alternatively we could simply allow the path signature to be optional in which case the wildcard is implied.

Sometimes, when a browser executes an ajax request from another server it performs a preflight check by sending the OPTIONS verb to the URL in question. We could automatically support this and respond with the following code:

        this.on("OPTIONS", "/", function(context) { context.send("OK"); });
        this.on("OPTIONS", "/.*", function(context) { context.send("OK"); });

Bug submitted on Electric Imp forums:

https://forums.electricimp.com/discussion/3817/rocky-contenttype-detection#latest

Hi,

Thank you for including libraries, its makes the imp code a lot more compact! Hopefully we'll have custom libraries soon

My browser (or jquery?) keeps on adding the encoding which does not work with the body parsing of Rocky 1.2.1

Can you change the contentType detection to always use find? It should then detect:
"application/json; charset=UTF-8"
"application/x-www-form-urlencoded; charset=UTF-8"
You currently do this for "multipart/form-data"

Thank you,
Johann

Incoming requests with the default content type header of application/x-www-form-urlencoded are not parsed correctly. Parsing results in a table with a key containing the request body and a blank value.

Scenario:

local numberOfRequests = 6;
local app = Rocky({"timeout": 6});
local count = 0;
app.on("GET", "/test", function(context) {
    // do nothing
}.bindenv(this));
imp.wakeup(0, function() {
    for (local i = 0; i < numberOfRequests; i++) {
        local req = http.request("GET", http.agenturl() + "/test", {}, "body");
        req.sendasync(function(res) {
            server.log("Response #" + (++count) + " | statuscode=" + res.statuscode);
        }.bindenv(this));
    }   
}.bindenv(this));
imp.wakeup(2, function() {
    Rocky.sendToAll(200, {"message": "OK"});
    imp.wakeup(2, function() {
        server.log("Total responses: " + count + "/" + numberOfRequests);
    }.bindenv(this));
}.bindenv(this));

Log:

[Agent] Response #1 | statuscode=200
[Agent] Response #2 | statuscode=200
[Agent] Response #3 | statuscode=200
[Agent] Response #4 | statuscode=200
[Agent] Total responses: 4/6
[Agent] Response #5 | statuscode=500
[Agent] Response #6 | statuscode=500

Tests:

This issue affects class: tests/AsyncRequests.agent.test.nut method: testMultipleAsyncRequestsWithSendToAll()

This would help to avoid so many runtime issues when someone accidentally creates two Rocky instances and handlers from the first one stop working.

Parse error 'slice out of range (line 391)

Rocky/Rocky.class.nut

Line 149 in 7add080

if ("content-type" in req.headers && req.headers["content-type"].slice(0,20) == "multipart/form-data;") {

- @imp-gino

Errors that are thrown in the callbacks for Rocky verb and .on methods can fail silently. Code inside the callback stops executing, but user is not notified that an error happened. When this happens it is very hard to track down the error.

Scenario:

local app = Rocky({"timeout": "invalid"});
app.onException(function(context, ex) {
    server.log("Got exception: " + ex);
    context.send(500);
});
app.on("GET", "/test", function(context) {
    context.send(200);
}.bindenv(this));
imp.wakeup(0, function() {
    local req = http.request("GET", http.agenturl() + "/test", {}, "body");
    req.sendasync(function(res){});
}.bindenv(this));

Log:

[Agent] ERROR: bad parameters to imp.wakeup(seconds, callback[, name])
[Agent] ERROR: from _onrequest agent_code:178

Tests:

This issue affects class: tests/RockyConstructor.agent.test.nut method: testRockyTimeoutOption()

Scenario:

local app = Rocky();
app.onException(function(context, ex) {
    server.log("Got exception: " + ex);
    context.send(500);
});
app.on("GET", "/test", function(context) {
    context.send(200);
}.bindenv(this), /*timeoutRoute*/"invalid");
imp.wakeup(0, function() {
    local req = http.request("GET", http.agenturl() + "/test", {}, "body");
    req.sendasync(function(res){});
}.bindenv(this));

Log:

[Agent] ERROR: bad parameters to imp.wakeup(seconds, callback[, name])
[Agent] ERROR: from _onrequest agent_code:178

Tests:

This issue affects class: tests/CoreRockyMethodNegative.nut method: testInvalidParamsTimeout()

Errors in handlers(Rocky.onTimeout, Rocky.onNotFound, Rocky.Route.onTimeout) are NOT caught by Rocky.onException, meanwhile errors in handlers(Rocky.authorize, Rocky.onUauthorized, Rocky.Route.authorize, Rocky.Route.onUnauthorized) are caught by Rocky.onException.

Tests:

This issue affects
class: tests/RockyHandlers.agent.test.nut method: testTimeoutException()
class: tests/RockyHandlers.agent.test.nut method: testNotFoundException()
class: tests/RockyRouteHandlers.agent.test.nut method: testTimeoutException()

Signatures are not case sensitive. Rocky converts all signatures to lower case. This becomes very evident when using Context.path to match a signature that contains upper case letters.

function handler(context) {
    context.send(200, "OK");
    
    local path = context.path;
    local p2 = split(context.req.path, "/");
    
    server.log("CONTEXT PATH: " + path[1]);
    server.log("REQUEST PATH: " + p2[1]);

    server.log("CONTEXT PATH MATCH: " + (path[1] == "RebootAgent"));
    server.log("REQUEST PATH MATCH: " + (p2[1] == "RebootAgent"));
}

_api.on("GET", "/meter/RebootAgent", handler);

When a web request is made using the path "/RebootAgent" the following logs are produced:
2018-10-18 13:06:12 -07:00 | [Agent] | CONTEXT PATH: rebootagent
2018-10-18 13:06:12 -07:00 | [Agent] | REQUEST PATH: RebootAgent
2018-10-18 13:06:12 -07:00 | [Agent] | CONTEXT PATH MATCH: false
2018-10-18 13:06:12 -07:00 | [Agent] | REQUEST PATH MATCH: true

The ability to configure the timeout on a route-level basis (that would take precedence over the global-level timeout) would be very handy. There are some requests that require an extended period of time on the device before completion is successful, and I'd rather not have to extend the timeout for the entire application for the purpose of serving a single route, and using the route-level onTimeout() handler to catch/hold the response seems like a hacky way to do it, too.

Currently the access control doesn't exposed any headers so CORS prevents anything but the most basic headers from being read when executed from another web server. If we had an array of headers to be exposed they could automatically be exposed in the addAccessControl method.

For example:

    rocky.addExposedHeader("X-Version");

or

    rocky.setExposedHeaders(["X-Version"]);

Would result in:

    res.header("Access-Control-Expose-Headers", "X-Version");

It would be nice to have cookies supported in Rocky (to automatically set the cookies path to that of the agent to properly scope it):

// If we had http.agentpath(), this would be less error-prone.
local URL = http.agenturl();
local AGENT_ID = split(URL, "/")[2];
// And note that it needs to be the agent *path*, rather than *id*
// because (thought experiment)
// what if we changed agents to live under the 
// '<agent_id>.agent.electricimp.io' domain?

http.onrequest(function(req, res) {
    // This issues cookies with path=/, which means that the browser
    // sends them to any page on agent.electricimp.com;
    // this means that other agents can read it.

    // DON'T:
    //res.header("set-cookie",
    //            "ID=6853b162f152; max-age=3600");

    // DO: Scope the cookie to the agent path.
    res.header("set-cookie",
                "ID=6853b162f152; max-age=3600; path=/" + AGENT_ID);

    res.send(200, "OK");
});