Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

update max user agent length. #42

Merged
merged 5 commits into from
Apr 16, 2019
Merged

Conversation

ShivangiReja
Copy link
Member

Description

Brief description of the changes made in the PR. This helps in making better change log

  • There's a hard limit of 128 characters on the user agent string and it could easily go over the limit( custom user agent string etc).
  • This pr updates the limit to 512 characters.

Reference to any github issues

@ShivangiReja ShivangiReja added the Client Issues that refer to the client sdk label Apr 15, 2019
@ShivangiReja ShivangiReja self-assigned this Apr 15, 2019
@@ -140,7 +140,7 @@ export module ConnectionContextBase {
const userAgent = parameters.connectionProperties.userAgent;
if (userAgent.length > Constants.maxUserAgentLength) {
throw new Error(
`The user-agent string cannot be more than 128 characters in length.` +
`The user-agent string cannot be more than 512 characters in length.` +
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use the constant maxUserAgentLength here instead of hard coding 512.

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Right after creating the connectionOptions object lets use log to log the same.

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Let's also update the version number so we can release the next version today

@ShivangiReja
Copy link
Member Author

@ramya-rao-a As discussed offline we will add a log for all the connection properties after successfully creating a connection in EventHubs.

@ramya-rao-a
Copy link
Contributor

@ShivangiReja Azure/azure-sdk-for-js#2270 was the fix done in the main repo to fix the auditing issue.
Please do the same here to get the CI check

@mikeharder
Copy link
Member

mikeharder commented Apr 16, 2019

@ramya-rao-a , @ShivangiReja : PR to update dependencies: #43

@ramya-rao-a ramya-rao-a merged commit 3892d07 into Azure:master Apr 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client Issues that refer to the client sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants