Query String not present in Request::uri()

[bernardobelchior]

Hey! I was trying to obtain values from the query string from a API GW request and I think I found a bug.

I was trying get the query string values by using Url::parse to parse the URI obtained from request.uri().to_string(). However the query string is not added to the URI when converting from LambdaRequest to Request. I believe this is not intended, but would like to confirm that.

Note 1: Eventually, I found the request.query_string_parameters() function, as mentioned in the examples.
Note 2: I also mentioned this problem in this issue.

[softprops]

Quick question, in your use case is this more of an inconvenience or a discovery issue?

A goal of using the http::Request type is to work with the existing http crate ecosystem, in that regard if you are using lambda http with another crate that does query string parsing this nuance of http lambda is an inconvenience.

A feature of api gateway is that these query
string parameters have already been preparsed for you meaning you'd end up needing to reparse what's already been parsed if you were to work with a raw http::Request uri. In that sense it may have been a case where discovery was an issue where it may not have been clear api gateway provides this for you and how that's exposed via lambda http.

I'm curious if it was one or the other or maybe an angle I'm not thinking of yet.

[bernardobelchior]

At first, it was a discovery issue. Eventually, I stumbled upon RequestExt and managed to get the query string.
Nevertheless, I still believe this is not the intended behavior. If I have a use case where I need the whole URL, I'd need to obtain it from request.uri() and then add the query string manually. However, this is not how the vanilla http::request works, so this seems like unintended behavior.

[seanpianka]

It would be helpful if there was an escape-hatch available to add the url-encoded query parameters back into the URI. For Lambda applications that include a separate HTTP routing library, the default behavior of removing the query parameters from the original URI is unexpected.

While there is a workaround to retrieve the query parameters via RequestExt, one needs to re-url-encode the query parameters from the StrMap and add them manually back into the URI. This seems messy and a supported alternative would be appreciated!

[seanpianka]

My workaround for converting the lambda_http::Request into hyper::Request:

async fn entrypoint(req: Request, _ctx: Context) -> Result<impl IntoResponse, Error> {
    let query_params = req.query_string_parameters();
    // Convert the lambda_http::Request into a hyper::Request, replacing the URI with the local
    // address of the routerify server.
    let (mut parts, body) = req.into_parts();
    let body = match body {
        Body::Empty => hyper::Body::empty(),
        Body::Text(t) => hyper::Body::from(t.into_bytes()),
        Body::Binary(b) => hyper::Body::from(b),
    };
    let mut uri = format!("http://{}{}", SERVER_ADDR, parts.uri.path());
    // AWS Lambda Rust Runtime will automatically parse the query params *and* remove those
    // query parameters from the original URI. This is fine if you're writing your logic directly
    // in the handler function, but for passing-through to a separate router library, we need to
    // re-url-encode the query parameters and place them back into the URI.
    if !query_params.is_empty() {
        uri.push('?');
        let mut serializer = url::form_urlencoded::Serializer::new(&mut uri);
        for (key, value) in query_params.into_iter() {
            serializer.append_pair(key, value);
        }
        serializer.finish();
    }
    parts.uri = match hyper::Uri::from_str(uri.as_str()) {
        Ok(uri) => uri,
        Err(e) => panic!(format!(
            "failed to convert local server addr + path to hyper URI: {:?}",
            e
        )),
    };
    let req = hyper::Request::from_parts(parts, body);
    // ...
}

[Mdrbhatti]

@bahildebrand could we have a new crates.io release soon including the changes from #395?

[bahildebrand]

I don't think there have been any breaking changes, so we could probably do a minor rev bump next week. Let me take a look and I'll get back to you.

[Mdrbhatti]

I believe #356 was a breaking change.