[Storage] [STG 101] Dynamic User Delegation SAS #44364
Conversation
weirongw23-msft
requested review from
annatisch,
jalauzon-msft and
vincenttran-msft
as code owners
| self._add_query(QueryStringConstants.SIGNED_REQUEST_HEADERS, "\n") | ||
| return | ||
| serialized = [str(k) + ":" + str(v) for k, v in request_headers.items()] | ||
| self._add_query(QueryStringConstants.SIGNED_REQUEST_HEADERS, "\n".join(serialized) + "\n") |
There was a problem hiding this comment.
Per the spec, request headers should have a trailing \n in addition to the \n separator for different signed values, and request query parameters should have a prefix \n (opposite to request headers).
There was a problem hiding this comment.
Are you sure it needs one at the end?
API Change CheckAPIView identified API level changes in this PR and created the following API reviews azure-storage-blob |
|
We still have to figure out the HttpPipelinePolicy to use for tests. As for the changelogs, I'm thinking: "Added support to specify request headers and query parameter values in generate_x_sas " |
| def add_request_headers(self, request_headers): | ||
| if not request_headers: | ||
| return | ||
| serialized = [str(k) + ":" + str(v) for k, v in request_headers.items()] |
There was a problem hiding this comment.
nit: This is already a Dict[str, str] right? So, you shouldn't need the str on the key and value.
| self._add_query(QueryStringConstants.SIGNED_REQUEST_HEADERS, "\n") | ||
| return | ||
| serialized = [str(k) + ":" + str(v) for k, v in request_headers.items()] | ||
| self._add_query(QueryStringConstants.SIGNED_REQUEST_HEADERS, "\n".join(serialized) + "\n") |
There was a problem hiding this comment.
Are you sure it needs one at the end?
| issued to the user specified in this value. | ||
| :param Dict[str, str] request_headers: | ||
| If specified, both the correct request header(s) and corresponding values must be present, | ||
| or the request will fail. |
There was a problem hiding this comment.
Can we make this description a little better? As is it doesn't make a lot of sense. Something like "Specifies a set of headers and their corresponding values that must be present in the request when using this SAS.". Somethng similar for query params.
This is internal doc, so it doesn't matter as much but please change the public docs on the public SAS functions. I didn't mention it before because we were just trying to get the API views done.
2 participants
No description provided.