Skip to content

Conversation

@ajainaus
Copy link
Contributor

@ajainaus ajainaus commented Jan 5, 2021

…odify CMakeList.txt to compile with 4221 warning

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@bretambrose bretambrose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some fundamental type mismatches (casting to chain) that have led to some messy consequences such that I haven't looked at the ntlm stuff much. Let's fix those and then simplify/rework all of the state logic and then I'll take a look again.

(void)num_headers;

/* SA-TBI: process CONNECT response headers here if needed */
//transfer the challenge to user area only when connect state in progress & sub state is not in progress
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this state logic at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added documentation/explanation in code

* User-supplied transform callback function that gets user data depending on callback state
*(example NTLM credentials/response)
*/
typedef char* (*aws_http_proxy_get_user_data_callback_fn)(int callback_state, void *userdata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need these callbacks to support async execution, as it is they can only be used synchronously. We'll want to pass the completion functions (terminate, continue) here as well and then it's the responsibilty of the callback implementer to invoke the right function when the data is ready.

enum aws_http_proxy_connection_type proxy_connection_type;

/* user token to use in kerberos authentication which is base64 encoded and provided by user*/
struct aws_byte_cursor user_token;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to pass in an (async-ready) function that fetches the user token rather than the user token itself. As it stands, I don't think this would support long-term connections because eventually the token expires and there's no sensible way to get a refreshed one.

@bretambrose bretambrose changed the base branch from ProxyRequestFlow to ProxyRequestFlowMerge January 20, 2021 21:22
@bretambrose bretambrose merged commit 170778b into awslabs:ProxyRequestFlowMerge Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants