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

The Regex at doesn't allow a trailing +, it only allows the a-zA-Z1-0 etc portion. #862

Closed
gaffo opened this issue Oct 7, 2018 · 4 comments

Comments

@gaffo
Copy link

gaffo commented Oct 7, 2018

https://github.com/awslabs/aws-cdk/blob/eb4bacfff68a2f2549572c5714057af01c12b7aa/packages/%40aws-cdk/aws-apigateway/lib/resource.ts#L149

This means you can't legally create {proxy+}, which is pretty important in api gateway.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 7, 2018

I believe we fixed this already?

@gaffo
Copy link
Author

gaffo commented Oct 7, 2018

if (!/^[a-zA-Z0-9._-]+$/.test(part)) {

doesn't seem like it would work, the only + in there is the repeat for the test pattern.

and it doesn't work on my 0.10.0 version I have on my machine.

However if I add either + to the [] section or (haven't tested) [+]? before $ it works.

@gaffo
Copy link
Author

gaffo commented Oct 7, 2018

My example stack is:

import cdk = require('@aws-cdk/cdk');
import apigateway = require('@aws-cdk/aws-apigateway');
import lambda = require('@aws-cdk/aws-lambda');

class MyStack extends cdk.Stack {
    constructor(parent: cdk.App, id: string, props?: cdk.StackProps) {
        super(parent, id, props);

        const lmbda = new lambda.Function(this, "Lambda", {
            code: lambda.Code.inline(`
def handler(a, b):
  return {
    "isBase64Encoded": False,
    "statusCode": 200,
    "headers": { },
    "body": "Hello, world!"
  }
`),
            handler: 'index.handler',
            runtime: lambda.Runtime.Python36,
        });

        const apigw = new apigateway.LambdaIntegration(lmbda, {
            proxy: true,
        });
        const api = new apigateway.RestApi(this, 'api', {
            defaultIntegration: apigw
        });

        api.root.addMethod('ANY');
        const all = api.root.addResource("{proxy+}");
        all.addMethod("ANY");
    }
}

class MyApp extends cdk.App {
    constructor(argv: string[]) {
        super(argv);

        new MyStack(this, 'hello-cdk', {
        	env: {
        		region: "us-west-2",
                account: "",
        	},
        });
    }
}

process.stdout.write(new MyApp(process.argv).run());

If the code is:

    if (!/^[a-zA-Z0-9\.\_\-]+$/.test(part)) {
        throw new Error(`Resource's path part only allow a-zA-Z0-9._- and curly braces at the beginning and the end: ${part}`);
    }

Then I get:

C:\Users\gaffo\cdk-test\node_modules\@aws-cdk\aws-apigateway\lib\resource.js:48
        throw new Error(`Resource's path part only allow a-zA-Z0-9._- and curly braces at the beginning and the end: ${part}`);
        ^

Error: Resource's path part only allow a-zA-Z0-9._- and curly braces at the beginning and the end: proxy+
    at validateResourcePathPart (C:\Users\gaffo\cdk-test\node_modules\@aws-cdk\aws-apigateway\lib\resource.js:48:15)
    at new Resource (C:\Users\gaffo\cdk-test\node_modules\@aws-cdk\aws-apigateway\lib\resource.js:9:9)
    at Object.addResource (C:\Users\gaffo\cdk-test\node_modules\@aws-cdk\aws-apigateway\lib\restapi.js:55:24)
    at new MyStack (C:\Users\gaffo\cdk-test\index.js:29:30)
    at new MyApp (C:\Users\gaffo\cdk-test\index.js:36:9)
    at Object.<anonymous> (C:\Users\gaffo\cdk-test\index.js:44:22)
    at Module._compile (module.js:641:30)
    at Object.Module._extensions..js (module.js:652:10)
    at Module.load (module.js:560:32)
    at tryModuleLoad (module.js:503:12)

If I change resource.ts to be:

    if (!/^[a-zA-Z0-9\.\_\-]+[\+]+$/.test(part)) {
        throw new Error(`Resource's path part only allow a-zA-Z0-9._- and curly braces at the beginning and the end: ${part}`);
    }

Then it works fine.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 9, 2018

Yes, this issue got fixed in #769. It's not part of the 0.10.0 release, but will be part of the next release.

@rix0rrr rix0rrr closed this as completed Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants