gremlin-rs icon indicating copy to clipboard operation
gremlin-rs copied to clipboard

Ability to add custom headers and query params

Open u382514 opened this issue 3 years ago • 2 comments

On the initial connect/upgrade we need the ability to add custom headers (think IAM/Sigv4 for Neptune or custom auth outside of basic) and the ability to add query params to the URL.

websocket::ClientBuilder has 'custom_headers' on the builder to do this and connect_async_with_tls_connector can this by supplying a full request.

Looks like adding query params can be completed by enhancing the websocket_url function.

Is there any reason we shouldn't go ahead with adding these features?

u382514 avatar Mar 30 '22 22:03 u382514

Hi @u382514

yes i guess if we take additional parameter to the ConnectionOptions like additional headers/query params we can then pass them to the underlying ClientBuilder

In case i can help on this :)

wolf4ood avatar Mar 31 '22 11:03 wolf4ood

@wolf4ood I think we should go one step further if you agree (while still keeping everything backwards compatible). The ability to add a URL (url::Url) to the connection options which would parse out the URL given and any query params it has. We can then backfill the host, port, query, path and credentials in the connection options for information purposes. Of course everything will continue to be backwards compatible by simply leaving url set to None and utilizing host, port, query, path and credentials params as usual.

Notes:

  • We will continue to set the protocol based on SSL option and ignore the protocol in URL. That way if they put in something like HTTPS we'll change it to WSS/WS respectively.
  • The websocket crates seem to set the Host header during its build and will overwrite any Host header provided in the custom headers. Usually this isn't a problem, however anybody using bastions with forwarding will possibly have issues if they don't know that. We can make a note of that in the documentation.
  • Query params in most packages allow for duplicate keys so we'll have to account for that in the Query struct to make it consistent.
  • Headers will also be a custom struct with helper functions to convert since some packages use HeaderMap while others use Headers and even their own structs.
  • Would like to change the websocket_url function to parse via URL and return the uri from the results rather than the format! macro. We can take advantage of URLs error handling for bad input. I don't think this would break anything from a backwards compatibility standpoint.

Let me know your thoughts.

pub struct ConnectionOptions {
    pub(crate) url: Option<Url>, // new variable
    pub(crate) host: String,
    pub(crate) port: u16,
    pub(crate) path: String, // new variable
    pub(crate) query: Option<Query>, // a new struct with some helper functions due to conversions
    pub(crate) headers: Option<Headers>, // a new struct with some helper functions due to conversions
    pub(crate) pool_size: u32,
    pub(crate) credentials: Option<Credentials>,
    pub(crate) ssl: bool,
    pub(crate) tls_options: Option<TlsOptions>,
    pub(crate) serializer: GraphSON,
    pub(crate) deserializer: GraphSON,
}

... 

fn default() -> ConnectionOptions {
        ConnectionOptions {
            url: None, 
            host: String::from("localhost"),
            port: 8182,
            query: None,
            path: String::from("/gremlin"),
            headers: None,
            pool_size: 10,
            credentials: None,
            ssl: false,
            tls_options: None,
            serializer: GraphSON::V3,
            deserializer: GraphSON::V3,
        }
    }

u382514 avatar Mar 31 '22 17:03 u382514