RhodesApp.cpp callback_redirect_to should not decode URL

[jtara]

See: rhomobile#1082

In RhodesApp.cpp, callback_redirect_to I think should not decode the URL - or, at least should not decode any included query string.

static void callback_redirect_to(void *arg, String const &strQuery )
{
    size_t nUrl = strQuery.find("url=");
    String strUrl;
    if ( nUrl != String::npos )
        strUrl = strQuery.substr(nUrl+4);

    if ( strUrl.length() == 0 )
        strUrl = "/app/";
	
    rho_http_redirect(arg, (rho::net::URI::urlDecode(strUrl)).c_str());
}

How I stumbled upon this: I have a barcode scan callback. It does some processing on barcode content, and then uses WebView Navigate to navigate the UI. params is a hash, which includes some decoded data as well as the original raw scan data.

In some cases, the raw data might contain some sequences that LOOK LIKE uri-encoded data. For example, this specific sequence: %96 url_for() properly encodes this as %2596 and then after decoding yields the original %96.

This gets internally redirected, like:

http://127.0.0.1:60607/system/redirect_to?url=http://127.0.0.1:60607/app/Scan/new_dgc?version=1&raw_data=HC1:6BF+...

The code above corrupts the query string by decoding what APPEARS to be uri-encoding but which is not. (For example, the + above will be turned into a space, and %96 will be turned into an invalid UTF-8 character and will be substituted with the substitution character.

  def scan_received
    Rho::Barcode.stop();
    if @params["status"] == "ok"
      scan = Scan.new
      t = Time.now
      scan.client_created_at = t
      scan.client_updated_at = t
      scan.was_preloaded = false
      scan.raw_data = @params['barcode']
      params = scan.decode
      WebView.navigate url_for(
        action: "new_#{scan.kind}",
        query: params,
        )
    else
      WebView.navigate Rho::Application.startURI
    end
  end

I fixed locally by changing the last line:

    //rho_http_redirect(arg, (rho::net::URI::urlDecode(strUrl)).c_str());
    rho_http_redirect(arg, strUrl.c_str());

But not sure if this is really a correct fix.

The code winds up here:

void rho_http_redirect( void* httpContext, const char* szUrl)
{
    HttpHeaderList headers;
    headers.push_back(HttpHeader("Location", szUrl));
    headers.push_back(HttpHeader("Content-Length", 0));
    headers.push_back(HttpHeader("Pragma", "no-cache"));
    headers.push_back(HttpHeader("Cache-Control", "must-revalidate"));
    headers.push_back(HttpHeader("Cache-Control", "no-cache"));
    headers.push_back(HttpHeader("Cache-Control", "no-store"));
    headers.push_back(HttpHeader("Expires", 0));
    headers.push_back(HttpHeader("Content-Type", "text/plain"));
    
    CHttpServer *serv = (CHttpServer *)httpContext;
    serv->send_response(serv->create_response("301 Moved Permanently", headers), true);
}

This seems perhaps vaguely relevant: curl/curl#473