yaws icon indicating copy to clipboard operation
yaws copied to clipboard

yaws_api:parse_query fails to find query parameters.

Open miby00 opened this issue 9 years ago • 5 comments

If I change:

%% parse the command line query data
parse_query(Arg) ->
    case get(query_parse) of
        undefined ->
            Res = case Arg#arg.querydata of
                      [] -> [];
                      D  -> parse_post_data_urlencoded(D)
                  end,
            put(query_parse, Res),
            Res;
        Res ->
            Res
    end.

to

%% parse the command line query data
parse_query(Arg) ->
    case get(query_parse) of
        _undefined -> %% <--- CHANGE
            Res = case Arg#arg.querydata of
                      [] -> [];
                      D  -> parse_post_data_urlencoded(D)
                  end,
            put(query_parse, Res),
            Res;
        Res ->
            Res
    end.

Yaws works just fine.

The above code optimization will only work if the same process never handles two different queries.

Scenario for repeating error:

  1. Do a "arg_rewrite" of an incoming query, add some query parameters, and send it to a .yaws file.
  2. Read query parameters in the .yaws file using the function yaws_api:parse_query/1, the query parameters will not be found.

I stumbled across this error when testing my code for automatic redirect the login page when accessing information requiring login.

The error was introduced in commit: a3edbee77324aa0a6e15b5c4084f45bf7f86d122

miby00 avatar Oct 26 '15 16:10 miby00

Hi,

Could you share your arg_rewrite module ? Because I cannot reproduce the problem. Here is the naive arg_rewrite module I used to do my tests:

-module(add_query_param).
-export([arg_rewrite/1]).
-include_lib("yaws/include/yaws_api.hrl").

arg_rewrite(#arg{}=Arg) ->
    Req  = Arg#arg.req,
    {abs_path, Path} = Req#http_request.path,
    Path1 = case string:chr(Path, $?) of
               0 -> Path ++ "?param=value";
               _ -> Path ++ "&param=value"
           end,
    Req1 = Req#http_request{path={abs_path, Path1}},
    Arg#arg{req=Req1}.

capflam avatar Oct 26 '15 21:10 capflam

Sorry my description wasn't enough to reproduce the error.

Here I have some stupid boiled down code to make it easy to reproduce.

arg_rewrite(Arg = #arg{req = Req}) ->
    {abs_path, Path} = Req#http_request.path,
    arg_rewrite(Arg, Path).

arg_rewrite(Arg = #arg{req = Req}, "/reproduce/error") ->
    yaws_api:parse_query(Arg), %% This will save wrong params in process dictionary
    Url = "/reproduceError.yaws?test1=first&test2=second",
    Arg#arg{req = Req#http_request{path = {abs_path, Url}}};
arg_rewrite(Arg, "/reproduceError.yaws" ++ _) ->
    Arg;

If you then create a file reproduceError.yaws that looks like:

<erl>
out(Arg)->
    case yaws_api:parse_query(Arg) of
        [] ->
            [{status, 500},
             {header, {content_type, "application/json"}},
             {header, {cache_control, "no-cache"}},
             {html, json2:encode({struct, [{"error", "Params lost in space"}]})}];
        Params ->
            [{status, 200},
             {header, {content_type, "application/json"}},
             {header, {cache_control, "no-cache"}},
             {html, json2:encode({struct, Params})}]
    end.
</erl>

This call will work:

http://localhost/reproduceError.yaws?test1=first&test2=second

and produce the following output:

{"test1":"first","test2":"second"}

This call will fail:

http://localhost/reproduce/error

with the following output:

{"error":"Params lost in space"}

If you remove the line

yaws_api:parse_query(Arg), %% This will save wrong params in process dictionary

everything works as intended.

In my more complex code I do not have a explicit yaws_api:parse_query, but something I do makes it happen anyway. But the above code will at least show you the problem.

If the above doesn't works for you we run different setups. But lets start with you trying the above code :)

Thx for a great open source effort!

miby00 avatar Oct 27 '15 08:10 miby00

Ok, I understand. I think you call yaws_api:getvar/2 or yaws_api:queryvar/2. These functions depend on yaws_api:parse_query/1. Query parsing is designed to be done in yaws scripts or appmod. But this is not mandatory. The API should be improved.

In the meantime, a workaround would be to erase previously parsed query by calling erase(query_parse) by hand in your arg_rewrite module. Of course, all parameters from the original request not found in the new one will be lost.

capflam avatar Oct 27 '15 09:10 capflam

Thanks for the quick response, I will try to work around the problem. I still think it would be nice if the problem was fixed :)

miby00 avatar Oct 27 '15 10:10 miby00

Of course, I will work on it. It's just a matter of time now :) I'm pretty busy at the moment.

capflam avatar Oct 27 '15 11:10 capflam