Skip to content

Conversation

@sohtsuka
Copy link
Contributor

Event data unexpectedly becomes empty when invoking an API request with content types
multipart/form-data, application/x-www-form-urlencoded or application-x-url-encoded.

$ cat template.yaml
AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Resources:
    HelloWorldFunction:
        Type: AWS::Serverless::Function
        Properties:
            CodeUri: hello_world/build/
            Handler: app.lambda_handler
            Runtime: python2.7
            Events:
                HelloWorld:
                    Type: Api
                    Properties:
                        Path: /hello
                        Method: post

$ cat hello_world/build/app.py
import json

def lambda_handler(event, context):
    return {
        "statusCode": 200,
        "body": event['body']
    }

$ # sam local start-api

$ cat test.txt
TEST

$ curl -d @test.txt http://localhost:3000/hello
no data 

The cause is that Flask request's data property does not contain the data comes with these content types. get_data() method always returns raw data and seems to be more relevant.

After the changes:

$ # samdev local start-api

$ curl -d @test.txt http://localhost:3000/hello
TEST

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

@billyshambrook
Copy link

This is needed to test slack commands.

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

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

What you have here looks great! My only ask would be for both a functional and integration test, so that way we can catch regressions in this area. I would pick one of the headers (multipart/form-data, application/x-www-form-urlencoded or application-x-url-encoded) and check the data in the event. The tests can go into the TestService_EventSerialization Functional test class and in the TestServiceRequests Integration test class. You can follow this integ test as an example.

@jfuss jfuss added stage/in-progress A fix is being worked on area/local/start-api sam local start-api command labels May 29, 2018
@sanathkr
Copy link
Contributor

@sohtsuka Any chance you can add the functional & integration tests that @jfuss requested? These tests will greatly improve the value of your change by ensuring this behavior is never broken in future :)

@sohtsuka
Copy link
Contributor Author

@jfuss @sanathkr Thanks for reviewing this. I'll try to add the tests.

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

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

One small comment on the integ tests. It is strictly for readability but feel it is important to request a change for.

"""
Form-encoded data should be put into the Event to Lambda
"""
response = requests.post(self.url + "/echobase64eventbody",
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this endpoint echoes the Event back but the path is specific to base64. For this case it wont affect the outcome but to keep things clean and readable, can you create a new path on the EchoEventFunction and update the path here.

Copy link
Contributor

@sanathkr sanathkr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

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

:)

@sanathkr
Copy link
Contributor

@sohtsuka I noticed this was your first PR. Congrats 🎉. Merged!

@sanathkr sanathkr merged commit 2a27dac into aws:develop Jun 14, 2018
hawflau pushed a commit that referenced this pull request May 9, 2022
* Dotnet6 support for samcli

* add nodejs16.x to test_workflow and architecture.py

* Fix the test tests/unit/commands/init/test_cli.py::TestCli::test_init_cli_node

* Fix test_init_cli_node by using a new dict instead of extra_context_as_json
hawflau added a commit that referenced this pull request May 9, 2022
* Dotnet6 support for samcli

* add nodejs16.x to test_workflow and architecture.py

* Fix the test tests/unit/commands/init/test_cli.py::TestCli::test_init_cli_node

* Fix test_init_cli_node by using a new dict instead of extra_context_as_json

Co-authored-by: Aboelhamd Aly <aboelha@amazon.com>
mildaniel added a commit that referenced this pull request May 30, 2022
* feat: updating app templates repo hash with (032e70a66732ed18db7ba6c8b13293cada3de740) (#3849)

Co-authored-by: GitHub Action <action@github.com>

* chore: Added Support for Test Account Management (#3845)

* Added get_testing_resources.py

* Added Setting Test Env Vars

* Added send-task-success to Windows

* Updated MANAGED_TEST_RESOURCE_STACK_NAME

* Fixed Region and Bash Env Var

* Added venv to get_testing_resources

* Fixed CredentialDistributionLambda region

* Added Cred Protection for Testing

* Fixed Python Version

* Added venv for pythonn3.9

* Fixed FunctionError Check

* Fixed Cred Key Names

* Fixed jq echo Syntax

* Removed test_env_var Debug Echo

* Added Docker Authentication and Cred Swap

* Added Debug STS Caller Identity

* Removed Debug STS

* Updated Lambda Max Attempts to Avoid Calling Multiple Times

* Fixed Ubuntu NoAvailableAccountException Check

* Added Exit to NoAvailableAccountException

* Updated to Exit Code Check

* Fixed Ubuntu jq and Windows Newlines

* Fixed Windows on_finish Newlines

* Removed Docker for Non Canary CI

* Updated to Echo Static Error Message

* Added Docs for get_testing_resources

* Fixed Ubuntu Send TaskToken

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* fix: command should not fail when global config fails to write (#3859)

* fix: don't fail when global config fails to write

* fix tests & change to warning message for failing to read or write

* Bump Lambda Builder version to 1.16.0 (#3865)

* feat: updating app templates repo hash with (22bb8ca9d29bea25f535c29a919fa4e963a2755b) (#3867)

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

* Nodejs16 support for samcli (#419) (#3866)

* Dotnet6 support for samcli

* add nodejs16.x to test_workflow and architecture.py

* Fix the test tests/unit/commands/init/test_cli.py::TestCli::test_init_cli_node

* Fix test_init_cli_node by using a new dict instead of extra_context_as_json

Co-authored-by: Aboelhamd Aly <aboelha@amazon.com>

* chore: Version bump to 1.49.0 (#3868)

* chore: Version bump to 1.49.0

* Fix init test for node

* Fix init test for node

* Black reformat

* fix: iac canaries (#3874)

* fix: iac canaries credentials

* add cleanup step as well

* install p39 venv

* install jq

* fix: Lock functions from building with same codeuri at the same time (#3862)

* Lock functions from building with same codeuri at the same time

* Remove unused import

* Remove bin. Add check for befor entering lock context

* Remove unneeded files, updated integration tests

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* feat: updating app templates repo hash with (70afc6988b77387818e3be7f0d2a836878099fb5) (#3869)

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix: Use abs path to upload stackset template on S3 (#3818)

* fix: Use abs path to upload stackset template on S3

* test: ✅ Add missing integration test for stack sets package

Co-authored-by: Massimo Maino <massimo.maino@claudacademy.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix: Add in trace event revision to prevent partial trace printing (#3876)

* Adding revision in traces to resolve partial traces

* Correcting file name

* Black reformatting

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* chore: bump version to 1.50.0 (#3878)

* feat: sam init support for x-ray tracing (API Gateway) (#3856)

* Set the global parameter Api.TracingEnabled to True to enable API Gateway X-Ray tracing when the Trace option is specified.

* passed unittest/make pr.

* reset make black

* create separate methods

* use generic function

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix: multivalue query params http (#3788)

* fixes multivalue query param requests for HTTP APIs

When a query parameter is specified multiple times in a URL, e.g.
`path?mykey=1&mykey=2&mykey=3`
this should create an event with all appropriate values.

Prior to this commit, when using `sam local start-api` with an HTTP API,
only the first value is present in the queryStringParameters of the
event. In payload format v2 [1] used in HTTP APIs, multiple values
should be combined with commas and included in the queryStringParameters
field. This commit fixes the behavior for query string parameters.

[1] https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html

* black reformat

* apply suggestion

* add test

Co-authored-by: Jason LaPier <jason@heyfieldday.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* feat: Nested stack support for sync watch (#3853)

* Nested stack support for sync watch

* Addressing comments

* Black reformatting

* Refactoring validator

* Change trigger creation failure from debug to warning

* Improve logging

* Changing validation behaviour to not fail initialization

* Refactoring

* Refactoring method names and updating docstring

Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>

* feat: sam traces integration test (#3618)

add traces integration test

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* chore: fixing the trace event class to support trace_id option (#3879)

* feat: updating app templates repo hash with (9f2cb3069f0ac3a21c2dec58adfb9450d26086e2) (#3880)

Co-authored-by: GitHub Action <action@github.com>

* feat: improve new PR and issue labeling for triaging (#3890)

* feat: improve new PR and issue labeling for triaging

* add 'anything else' template and disable blank templates, in order to ensure they are all auto-labeled for triaging

* anything else -> other

* feat: nested stack support for sam logs (#3881)

* feat: nested stack support for sam logs

* update command documentation

* update to use key value pairs instead of list of resources

* update expected values for infra sync integ tests

* feat: updating app templates repo hash with (6d61039f51b2d12e2f9e339eba0c023ddbf395ae) (#3892)

Co-authored-by: GitHub Action <action@github.com>

* feat: prevent deploy throttle with customizable interval for DescribeStack via env (#3500)

* Customize deploy describe stack sleep interval via environment

* Test deployer with custom describe stack interval

* Move and rename env var for deploy polling

* Fix tests on deploy poller

* Move env var from context to command

* Fix tests for command

* Reset retry_attemps on success after exception

Otherwise if there is an exceptions,
every next iteration won't sleep and
it will spam the server

* Add details on what to set in the deploy help text output.

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com>
Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com>

* feat: make java build scope changes default (#3896)

* feat: make java build scope changes default

* update lambda builders version

* Add explicit --no-cached option  (#3889)

* added --no-cached option to override cached = true in samconfig.toml

* Added test to check that the no-cached option overrides the cached = true setting in samconfig.toml

* Added integration tests to verify the --no-cached option

* Reformatted file

* Modified integration test and testdata path source

Co-authored-by: Andrew Zhan <zhandr@amazon.com>

* feat: ADL nested stack support (#3893)

* feat: add nested stack support for auto dependency layer

* add/update unit tests

* address comments

* use different variable name for sanitized function logical id

Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>

* test: Integration Tests for Sync Watch with Nested Stacks (#3909)

* Generalize setup of existing watch tests

* Add tests for infra nested stacks

* Add tests for code sync watch

* Paramaterize ADL config

* Black reformat

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix: Add lockchain and status check to prevent sync flows race condition on function update (#3905)

* Initial investigation to fix

* Add lockchain and status check to prevent function config update collision with function update

* Adding in unit tests

* Black reformatting

* Improving unit test

* Add integration test

* Refactoring to address comments

* Ignore type check for boto client call

* Refactoring

* Added lockchain and wait mechanism for image functions

* Improving docstring

* Sync code nested (#3887)

add nested stack support for sam sync code

* fix: correct node16 debugging configuration (#3913)

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* feat: new release of RIE: 1.6 (#3897)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Wilton_ <CoshUS@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Aboelhamd Aly <aboelha@amazon.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: Massimo Maino <maintux@gmail.com>
Co-authored-by: Massimo Maino <massimo.maino@claudacademy.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>
Co-authored-by: iwasa <Tak1wa@users.noreply.github.com>
Co-authored-by: Jason LaPier <jason@heyfieldday.com>
Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com>
Co-authored-by: Ruperto Torres <86501267+torresxb1@users.noreply.github.com>
Co-authored-by: Mickael Bourgois <35171168+BourgoisMickael@users.noreply.github.com>
Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com>
Co-authored-by: Andrew Zhan <zhandr@amazon.com>
Co-authored-by: JadenSimon <31319484+JadenSimon@users.noreply.github.com>
Co-authored-by: Renato Valenzuela <37676028+valerena@users.noreply.github.com>
andrew-zhan139 pushed a commit to andrew-zhan139/aws-sam-cli that referenced this pull request May 31, 2022
* Dotnet6 support for samcli

* add nodejs16.x to test_workflow and architecture.py

* Fix the test tests/unit/commands/init/test_cli.py::TestCli::test_init_cli_node

* Fix test_init_cli_node by using a new dict instead of extra_context_as_json

Co-authored-by: Aboelhamd Aly <aboelha@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/local/start-api sam local start-api command stage/in-progress A fix is being worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants