Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tasks.DynamoAttributeValue().withB double encoding value? #7724

Closed
eikeon opened this issue May 1, 2020 · 12 comments
Closed

tasks.DynamoAttributeValue().withB double encoding value? #7724

eikeon opened this issue May 1, 2020 · 12 comments
Assignees
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@eikeon
Copy link

eikeon commented May 1, 2020

I'm passing in a binary value to a step function which ends up getting base64 encoded somewhere along the way and it ends up getting base64 encoded a second time when I'm using withB as below. It's unclear from the current documentation if the string passed to withB should be base64 encoded already but it's type and the example seem to suggest it. If it's supposed to be a string of the raw binary is there a way decode an already base64 encoded string from the TaskInput.

The code I'm using to pass this value in as binary has been used in a couple other places using a couple different languages etc on the receiving end without issue. And no where am I knowingly encoding it a second time within the CDK state machine here.

withB's documentation shows

withB(value)🔹

public withB(value: string): DynamoAttributeValue
Parameters

value string
Returns

DynamoAttributeValue
Sets an attribute of type Binary.

For example: "B": "dGhpcyB0ZXh0IGlzIGJhc2U2NC1lbmNvZGVk"

Reproduction Steps

    new tasks.DynamoUpdateItem({
        tableName: props.content.tableName,
        partitionKey: {
            name: "ContentID",
            value: new tasks.DynamoAttributeValue().withB(
                sfn.TaskInput.fromDataAt("$.ContentID").value
            ),
        },
        ...

Error Log

Environment

  • CLI Version : 1.36.1 (build 4df7dac)
  • Framework Version: 1.36.1 (build 4df7dac)
  • OS : macOS Catalina 10.15.5 Beta (19F62f)
  • Language : Typescript

Other


This is 🐛 Bug Report

@eikeon eikeon added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 1, 2020
@eikeon
Copy link
Author

eikeon commented May 2, 2020

To work around this I'm taking the base64 encoded string (that is resulting from me passing in binary to the state machine without my base64 encoding it) and decoding it, and using the binary toString. For example, I run this extra task

        const contentIDB = new sfn.Task(this, "Decode ContentID", {
            task: new tasks.EvaluateExpression({
                expression:
                    "Buffer.from($.ContentID, 'base64').toString()",
            }),
            resultPath: "$.ContentIDB",
        });

and then use $.ContentIDB instead of $.ContentID

@eikeon
Copy link
Author

eikeon commented May 2, 2020

They are only starting with the right first few characters after that attempted work around.

For example should be the same length as:
AQERFArszmn0KWJPnNOEEAewUpzQXvee
So it's now starting off right but is too long:
AQERFDDvv73vv70fNGXvv71V77+9ee+/vdeNBu+/ve+/vVLvv70Pbw==

@eikeon
Copy link
Author

eikeon commented May 2, 2020

I need to add encoding='binary' to the above toString for this to work. But then I get

{
  "error": "ReferenceError",
  "cause": {
    "errorType": "ReferenceError",
    "errorMessage": "encoding is not defined",
    "trace": [
      "ReferenceError: encoding is not defined",
      "    at eval (eval at handler (/var/task/index.js:8:12), <anonymous>:1:76)",
      "    at Runtime.handler (/var/task/index.js:8:12)",
      "    at Runtime.handleOnce (/var/runtime/Runtime.js:66:25)"
    ]
  }
}

Which works here docker run --rm -it --entrypoint node -v "$PWD":/var/task:ro,delegated lambci/lambda:nodejs10.x -p "Buffer.from('AQERFArszmn0KWJPnNOEEAewUpzQXvee', 'base64').toString(encoding='binary')"

Guessing EvaluateExpression is trying to do something with the encoding= bit

@eikeon
Copy link
Author

eikeon commented May 2, 2020

Will try to dive into CDK code to see how one might allow for passing in the already base64 encoded string to the withB

@eikeon
Copy link
Author

eikeon commented May 2, 2020

Adding the encoding='binary' positionally toString('binary') in the above workaround results in:
AQERFArDrMOOacO0KWJPwpzDk8KEEAfCsFLCnMOQXsO3wp4
vs the expected
AQERFArszmn0KWJPnNOEEAewUpzQXvee

closer... another couple characters. Perhaps the wrong flavor of base64 in this work around

@eikeon
Copy link
Author

eikeon commented May 2, 2020

CDK seems to be doing its part. It's generating the following bit in the state machine definition:

        "Record FetchCompleted": {
            "Next": "Publish FetchCompleted",
            "Parameters": {
                "Key": { "ContentID": { "B.$": "$.ContentID" } },
                "TableName": "content-LabelsBA41E4AA-1XBL4K17A6P60",
                "ExpressionAttributeValues": {
                    ":now": { "N.$": "$.FetchCompletedTime" }
                },
                "UpdateExpression": "SET FetchCompletedTime = :now REMOVE FetchStartedTime"
            },
            "Type": "Task",
            "Resource": "arn:aws:states:::dynamodb:updateItem",
            "ResultPath": "$.RecordFetchCompleted"
        },

Is there somewhere to file a bug against the step function integration services? Where is should be interpreting that "$.ContentID" as a base64 encoded string and converting it to a blob that's expected there

@shivlaks
Copy link
Contributor

shivlaks commented May 4, 2020

hey @eikeon I pinged @wong-a who might have better insight here on the Step Functions behaviour you're observing

@eikeon
Copy link
Author

eikeon commented May 4, 2020

@shivlaks Thank you; In the meantime I'm switching to a Lambda for this bit. But can help retest or keep digging if need be.

@eikeon
Copy link
Author

eikeon commented May 5, 2020

Just reporting back that using a lambda for this task instead of the DynamoUpdateItem step functions service integration worked to avoid this issue. And I can have the lambda grab the current time instead of relying on a separate EvaluateExpression task.

@wong-a
Copy link
Contributor

wong-a commented May 6, 2020

In the AWS SDKs, you can pass a buffer (e.g. ByteBuffer in Java and Buffer in JavaScript) and convert to base64 for you. You only need to encode the data yourself if you're using the DynamoDB Low-level API.

Since Amazon States Language is JSON-based, binary data can only be encoded as a string. What's probably happening here is that Step Functions is deserializing the string which is getting encoded again by the AWS SDK. I'll bring this up with my team so we can take a deeper look at this.

The documentation is kind of misleading as it's not clear that the data is going to be encoded again. If you pass an unencoded string (e.g. Hello World) in the ASL definition, the stored value is base64 encoded (SGVsbG8gV29ybGQ=). If you pass that as encoded a base64 encoded string, the stored value is encoded again in base64 (U0dWc2JHOGdWMjl5YkdRPQ==).

@eikeon It sounds like you're expecting The binary attribute value to behave like the Low-level API with base64 strings passed directly to DynamoDB without further encoding, is that correct?

@eikeon
Copy link
Author

eikeon commented May 6, 2020

@wong-a Yes, I was expecting the binary value to behave like the low-level API. And since we’re in this JSON-based context passing binary with type string — I assumed it’d need to be expecting base64 encoded string for it to work.

I’ve been passing the binary around with the various AWS SDKs as you describe. And by the time the data gets to were I was trying to use tasks.DynamoUpdateItem above within the state machine the ContentID is already base64 encoded since ASL is JSON-based. Normally if the SDKs encode something on the way in they decode it on the way out or the other way around.

@nija-at
Copy link
Contributor

nija-at commented May 13, 2020

Since this is not a CDK issue, I'm resolving this issue. Let us know if there's anything we can do on the CDK side to make this better.

@wong-a internal tracking: t.corp/D16302653

@nija-at nija-at closed this as completed May 13, 2020
mergify bot pushed a commit that referenced this issue Mar 13, 2023
…k/lambda-layer-awscli (#24590)

Bumps [awscli](https://github.com/aws/aws-cli) from 1.27.84 to 1.27.89.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/aws/aws-cli/blob/develop/CHANGELOG.rst">awscli's changelog</a>.</em></p>
<blockquote>
<h1>1.27.89</h1>
<ul>
<li>api-change:<code>ivschat</code>: This release adds a new exception returned when calling AWS IVS chat UpdateLoggingConfiguration. Now UpdateLoggingConfiguration can return ConflictException when invalid updates are made in sequence to Logging Configurations.</li>
<li>api-change:<code>secretsmanager</code>: The type definitions of SecretString and SecretBinary now have a minimum length of 1 in the model to match the exception thrown when you pass in empty values.</li>
</ul>
<h1>1.27.88</h1>
<ul>
<li>api-change:<code>codeartifact</code>: This release introduces the generic package format, a mechanism for storing arbitrary binary assets. It also adds a new API, PublishPackageVersion, to allow for publishing generic packages.</li>
<li>api-change:<code>connect</code>: This release adds a new API, GetMetricDataV2, which returns metric data for Amazon Connect.</li>
<li>api-change:<code>evidently</code>: Updated entity override documentation</li>
<li>api-change:<code>networkmanager</code>: This update provides example usage for TransitGatewayRouteTableArn.</li>
<li>api-change:<code>quicksight</code>: This release has two changes: add state persistence feature for embedded dashboard and console in GenerateEmbedUrlForRegisteredUser API; add properties for hidden collapsed row dimensions in PivotTableOptions.</li>
<li>api-change:<code>redshift-data</code>: Added support for Redshift Serverless workgroup-arn wherever the WorkgroupName parameter is available.</li>
<li>api-change:<code>sagemaker</code>: Amazon SageMaker Inference now allows SSM access to customer's model container by setting the &quot;EnableSSMAccess&quot; parameter for a ProductionVariant in CreateEndpointConfig API.</li>
<li>api-change:<code>servicediscovery</code>: Updated all AWS Cloud Map APIs to provide consistent throttling exception (RequestLimitExceeded)</li>
<li>api-change:<code>sesv2</code>: This release introduces a new recommendation in Virtual Deliverability Manager Advisor, which detects missing or misconfigured Brand Indicator for Message Identification (BIMI) DNS records for customer sending identities.</li>
</ul>
<h1>1.27.87</h1>
<ul>
<li>api-change:<code>athena</code>: A new field SubstatementType is added to GetQueryExecution API, so customers have an error free way to detect the query type and interpret the result.</li>
<li>api-change:<code>dynamodb</code>: Adds deletion protection support to DynamoDB tables. Tables with deletion protection enabled cannot be deleted. Deletion protection is disabled by default, can be enabled via the CreateTable or UpdateTable APIs, and is visible in TableDescription. This setting is not replicated for Global Tables.</li>
<li>api-change:<code>ec2</code>: Introducing Amazon EC2 C7g, M7g and R7g instances, powered by the latest generation AWS Graviton3 processors and deliver up to 25% better performance over Graviton2-based instances.</li>
<li>api-change:<code>lakeformation</code>: This release adds two new API support &quot;GetDataCellsFiler&quot; and &quot;UpdateDataCellsFilter&quot;, and also updates the corresponding documentation.</li>
<li>api-change:<code>mediapackage-vod</code>: This release provides the date and time VOD resources were created.</li>
<li>api-change:<code>mediapackage</code>: This release provides the date and time live resources were created.</li>
<li>api-change:<code>route53resolver</code>: Add dual-stack and IPv6 support for Route 53 Resolver Endpoint,Add IPv6 target IP in Route 53 Resolver Forwarding Rule</li>
<li>api-change:<code>sagemaker</code>: There needs to be a user identity to specify the SageMaker user who perform each action regarding the entity. However, these is a not a unified concept of user identity across SageMaker service that could be used today.</li>
</ul>
<h1>1.27.86</h1>
<ul>
<li>bugfix:eks: Output JSON only for user entry in kubeconfig fixes <code>[#7719](aws/aws-cli#7719) &lt;https://github.com/aws/aws-cli/issues/7719&gt;</code><strong>, fixes <code>[#7723](aws/aws-cli#7723) &lt;https://github.com/aws/aws-cli/issues/7723&gt;</code></strong>, fixes <code>[#7724](aws/aws-cli#7724) &lt;https://github.com/aws/aws-cli/issues/7724&gt;</code>__</li>
<li>api-change:<code>dms</code>: This release adds DMS Fleet Advisor Target Recommendation APIs and exposes functionality for DMS Fleet Advisor. It adds functionality to start Target Recommendation calculation.</li>
<li>api-change:<code>location</code>: Documentation update for the release of 3 additional map styles for use with Open Data Maps: Open Data Standard Dark, Open Data Visualization Light &amp; Open Data Visualization Dark.</li>
</ul>
<h1>1.27.85</h1>
<ul>
<li>api-change:<code>account</code>: AWS Account alternate contact email addresses can now have a length of 254 characters and contain the character &quot;|&quot;.</li>
<li>api-change:<code>ivs</code>: Updated text description in DeleteChannel, Stream, and StreamSummary.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/aws/aws-cli/commit/df68d39f402e023dd0ee48e6974eda850b4bbb21"><code>df68d39</code></a> Merge branch 'release-1.27.89'</li>
<li><a href="https://github.com/aws/aws-cli/commit/be4afd12701baadf08a343553b99975a2c02b7a0"><code>be4afd1</code></a> Bumping version to 1.27.89</li>
<li><a href="https://github.com/aws/aws-cli/commit/9583dc5226fdd93d752bcf319736af7c42602963"><code>9583dc5</code></a> Update changelog based on model updates</li>
<li><a href="https://github.com/aws/aws-cli/commit/7808d95fb9d1d99e0e0088132296e96d99a6a958"><code>7808d95</code></a> Merge branch 'release-1.27.88'</li>
<li><a href="https://github.com/aws/aws-cli/commit/7e4d04f4e522737ba951995886fe58e33a410ca3"><code>7e4d04f</code></a> Merge branch 'release-1.27.88' into develop</li>
<li><a href="https://github.com/aws/aws-cli/commit/7b64318189b3a3f2369885122b3249a961930b62"><code>7b64318</code></a> Bumping version to 1.27.88</li>
<li><a href="https://github.com/aws/aws-cli/commit/1c8486baccbfe705230ccf87850f8dfe5aba7ff3"><code>1c8486b</code></a> Update changelog based on model updates</li>
<li><a href="https://github.com/aws/aws-cli/commit/b40f487fa9a27ccf00e93e480bd7f73419d5f66a"><code>b40f487</code></a> Merge branch 'release-1.27.87'</li>
<li><a href="https://github.com/aws/aws-cli/commit/19b76b4b57f86ed4b6341a353d56cacf4f3582fd"><code>19b76b4</code></a> Merge branch 'release-1.27.87' into develop</li>
<li><a href="https://github.com/aws/aws-cli/commit/753b87077b1a8d21082cf2de1e48e9f4eec22352"><code>753b870</code></a> Bumping version to 1.27.87</li>
<li>Additional commits viewable in <a href="https://github.com/aws/aws-cli/compare/1.27.84...1.27.89">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=awscli&package-manager=pip&previous-version=1.27.84&new-version=1.27.89)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

4 participants