blobclient.upload() method works fine in version 12.18.0 but same code breaks in 12.23.0 of lib @azure/storage-blob

[shashikumarpandey]

• @azure/storage-blob:
• 12.23.0:
• Operating system:
• [] nodejs
  • 18.17.1: or greater
• [] browser
  • any:
• [] typescript
  • any:
• Is the bug related to documentation in
  • README.md
  • source code documentation
  • SDK API docs on https://docs.microsoft.com

Describe the bug
upload of json having special German characters works fine in library version 12.18.0 but remove few characters from last in lib version 12.23.0 version.

To Reproduce
Steps to reproduce the behavior:

1. Not working version of code

  async upload(tenantId: string, text: string, path: string): Promise<void> {
    const credential: AzureBlobCredential = await this.blobClientHelper.getCredential(tenantId);
    const blobConnectionString = await this.blobClientHelper.getConnectionString(credential);
    const blockBlobClient: BlockBlobClient = await this.blobClientHelper.getBobClient(
      blobConnectionString,
      credential.container_name,
      path
    );
   text = "german character containing special characters"; // use below highlighted data in bold and Italic
    await blockBlobClient.upload(text, text.length); // **this code works in 12.18.0 but not in 12.23.0**
  }

'{"plant":"1050","routing":"100007245514","routingType":"SHOP_ORDER","version":"SYS001","currentVersion":true,"status":"RELEASABLE","description":"000000000002120998","relaxedFlow":false,"entryRoutingStepId":"0100","quantityValidation":true,"automaticGoodsReceipt":false,"routingSteps":[{"stepId":"0100","sequence":10,"description":"Mat. Bereitstellung","entry":true,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0100","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1900"},"reportingStep":"0100","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0200"],"routingStepComponentList":[{"bomComponent":{"bom":{"bom":"100007245514-000000000002120998-1-1","version":"SYS001","bomType":"SHOPORDERBOM","plant":"1050"},"material":{"plant":"1050","material":"000000000000481023","version":"SYS001"},"sequence":10},"sequence":1,"quantity":40},{"bomComponent":{"bom":{"bom":"100007245514-000000000002120998-1-1","version":"SYS001","bomType":"SHOPORDERBOM","plant":"1050"},"material":{"plant":"1050","material":"000000000002240856","version":"ERP002"},"sequence":20},"sequence":2,"quantity":27}]},{"stepId":"0200","sequence":20,"description":"Wkz voreinstellen, überprüfen","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0200","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1650"},"reportingStep":"0200","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0400"],"routingStepComponentList":[]},{"stepId":"0400","sequence":30,"description":"NC-Fr.: 1ter Step","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0400","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1594"},"reportingStep":"0400","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0500"],"routingStepComponentList":[]},{"stepId":"0500","sequence":40,"description":"NC-Fr.: 2ter Step","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0500","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1594"},"reportingStep":"0500","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["9000"],"routingStepComponentList":[]},{"stepId":"9000","sequence":50,"description":"Abschluß Datenbank","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-9000","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1717"},"reportingStep":"9000","controlKey":{"controlKey":"ZO18"},"lastReportingStep":true,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","routingStepComponentList":[]}],"customValues":[],"routingOperationGroups":[{"routingOperationGroup":"100007245514-0-0100","description":"Mat. Bereitstellung","routingOperationGroupSteps":[{"routingStep":{"stepId":"0100","sequence":10,"description":"Mat. Bereitstellung","entry":true,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0100","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1900"},"reportingStep":"0100","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0200"],"routingStepComponentList":[{"bomComponent":{"bom":{"bom":"100007245514-000000000002120998-1-1","version":"SYS001","bomType":"SHOPORDERBOM","plant":"1050"},"material":{"plant":"1050","material":"000000000000481023","version":"SYS001"},"sequence":10},"sequence":1,"quantity":40},{"bomComponent":{"bom":{"bom":"100007245514-000000000002120998-1-1","version":"SYS001","bomType":"SHOPORDERBOM","plant":"1050"},"material":{"plant":"1050","material":"000000000002240856","version":"ERP002"},"sequence":20},"sequence":2,"quantity":27}]}}]},{"routingOperationGroup":"100007245514-0-0200","description":"Wkz voreinstellen, überprüfen","routingOperationGroupSteps":[{"routingStep":{"stepId":"0200","sequence":20,"description":"Wkz voreinstellen, überprüfen","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0200","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1650"},"reportingStep":"0200","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0400"],"routingStepComponentList":[]}}]},{"routingOperationGroup":"100007245514-0-0400","description":"NC-Fr.: 1ter Step","routingOperationGroupSteps":[{"routingStep":{"stepId":"0400","sequence":30,"description":"NC-Fr.: 1ter Step","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0400","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1594"},"reportingStep":"0400","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["0500"],"routingStepComponentList":[]}}]},{"routingOperationGroup":"100007245514-0-0500","description":"NC-Fr.: 2ter Step","routingOperationGroupSteps":[{"routingStep":{"stepId":"0500","sequence":40,"description":"NC-Fr.: 2ter Step","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-0500","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1594"},"reportingStep":"0500","controlKey":{"controlKey":"ZO21"},"lastReportingStep":false,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","nextStepList":["9000"],"routingStepComponentList":[]}}]},{"routingOperationGroup":"100007245514-0-9000","description":"Abschluß Datenbank","routingOperationGroupSteps":[{"routingStep":{"stepId":"9000","sequence":50,"description":"Abschluß Datenbank","entry":false,"routingOperation":{"stepType":"NORMAL","operationActivity":{"operationActivity":"100007245514-0-9000","version":"SYS001"},"weighRelevant":false,"automaticGoodsReceipt":false,"customValues":[]},"workCenter":{"workCenter":"1717"},"reportingStep":"9000","controlKey":{"controlKey":"ZO18"},"lastReportingStep":true,"rework":false,"queueDecisionType":"COMPLETING_OPERATOR","erpSequence":"0","routingStepComponentList":[]}}]}],"modifiedDateTime":"2024-06-06T15:24:07.123Z","createdDateTime":"2024-06-06T14:32:58.017Z","tenantId":"2479be7b-eac8-4d50-aef9-64e32f1bcc27"}'

Expected behavior
It should upload the json without corrupting the data
with text length in byte with utf8 encoding it works in 12.23.0 version(see below code). but our issue is more towards how it was working in 12.18.0 version and why all of a sudden it is not working with plain text.length. Since it involves a huge amount of data that are impacted, we need this info.

  const lengthInBytes = Buffer.byteLength(text, 'utf8');
  console.log(`Length in bytes: ${lengthInBytes}`); 

Additional context
Since this behavior is now changed it has impacted millions of record in our DB . also if you can explain with while passing text.length as a parameter in upload method . how it was handling it's text encoding . this will help us in understanding whether our old data is also corrupted or only data created between today and the day we upgrade our library are corrupted. please check the details for more information official Microsoft Support request number: 2406130030005435

[xirzec]

Looking into this, the method documentation does clearly state the length parameter is expected to be in bytes: https://learn.microsoft.com/en-us/javascript/api/@azure/storage-blob/blockblobclient?view=azure-node-latest#@azure-storage-blob-blockblobclient-upload

This has not changed since 12.18.0, but brings up an interesting question of why the issue was not previously seen. The main difference between the two versions in this issue is that 12.18.0 uses a package called @azure/core-http to make HTTP requests. This package in turn depends on node-fetch, which performs the actual call.

Looking at the source for node-fetch, we can see that they always calculate the Content-Length header and set it, as long as they are not given a stream:

https://github.com/node-fetch/node-fetch/blob/8b3320d2a7c07bce4afc6b2bf6c3bbddda85b01f/src/request.js#L250

Which means that node-fetch was overriding the Content-Length header set by the BlockBlobClient to be the correct byte length, even though the wrong value was passed in.

12.23.0 on the other hand uses @azure/core-rest-pipeline which only sets the Content-Length header when it is not already set by the client:

azure-sdk-for-js/sdk/core/core-rest-pipeline/src/nodeHttpClient.ts

Line 102 in b0b7d0f

if (body && !request.headers.has("Content-Length")) {

This is arguably more correct but allowed your invalid length parameter to pass through to the underlying transport, presumably causing the Storage service to ignore bytes sent after the provided length.

One fix here would be to have BlockBlobClient check when given a string input if the length is correct or not and either throw an Error or silently correct the value. @EmmaZhu do you have a preference?

[EmmaZhu]

I can reproduce the issue with uploading string like: "Wkz voreinstellen, überprüfen".

As the change is in dependencies for all SDKs, we may need to have a general fix for all storage SDKs.

[xirzec]

@EmmaZhu to clarify, you believe that we should handle the case of invalid Content-Length for determinate size body values (e.g. strings, ArrayBuffers, and so on) being passed by the user to operations which have body length as a parameter across all Storage packages?

Do you think we should continue to silently correct the mistake as before or should we begin to throw so that developers know they are passing an incorrect value?

[EmmaZhu]

Hi @xirzec ,

The issue only happens with OAuth token credentials.

We have following code in the StorageSharedKeyCredentialPolicy to set correct body length value to Content-Length header:

if (
      request.body &&
      (typeof request.body === "string" || (request.body as Buffer) !== undefined) &&
      request.body.length > 0
    ) {
      request.headers.set(HeaderConstants.CONTENT_LENGTH, Buffer.byteLength(request.body));
    }

I think it might be better to keep behavior consistent between different types of credentials.

[xirzec]

Oh interesting. I didn't realize the shared key credential had this behavior inside of it. I'm assuming it was something to do with being able to sign the request body correctly?

At any rate, I agree we should do this in a place not tied to credential type, perhaps we could simply pull it out into its own policy?

[timotheeguerin]

Also got into this issue here microsoft/typespec#3790, migrated to using Buffer.from and uploadData which seems to resolve the issue
Note that the doc does show to use text.length to pass to this parameter https://learn.microsoft.com/en-us/azure/storage/blobs/storage-blob-upload-javascript#upload-a-block-blob-from-a-string

Also not really sure why we even need to pass the content length and is not something the SDK can figure out from the text we pass