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

Dynamodb 20120810 call scan body is empty #7

Open
hugosenari opened this issue Feb 3, 2022 · 9 comments
Open

Dynamodb 20120810 call scan body is empty #7

hugosenari opened this issue Feb 3, 2022 · 9 comments

Comments

@hugosenari
Copy link

Hi, again.

I found a possible bug in Dynamodb 20120810 scan.
When I call it, body of Recallable is empty.

  let
    path: JsonNode = nil
    query: JsonNode = nil
    header: JsonNode = nil
    formData: JsonNode = nil
    body = %* {"TableName": "ReSSt"}
    request = scan.call(path, query, header, formData, body)
    response = waitfor request.issueRequest()
  echo "payload ", request.body # is empty
  echo "status ", response.status # error 400 invalid request

The problems is that atozHook is expecting a jString but scan.call expects body as jObject

@disruptek
Copy link
Owner

disruptek commented Feb 3, 2022

I run into this frequently. The main cause is a change I made a long time ago to appease one user or another; I didn't really think it through with respect to atoz and the behavior of AWS APIs varies enough that it causes problems in practice.

I have a 1000 line dynamo API built on atoz and it's full of garbage like this:

  import dynamodb_20120810 as ddb
  # imagine this getItem call;
  # let's call `js` our JObject holding the query...
  js["TableName"] = newJString table.name
  js["Key"] = key.toDynamo
  js["ConsistentRead"] = newJBool true
  # ...
  ddb.getItem.call(path = nil, _ = $js)

It's not pretty, but what's tricky about this stuff is doing the right thing across all over the AWS APIs without changing the openapi library to the point where it doesn't work correctly.

Or it's possible that I'm completely wrong about what the problem is. But, try this approach first and let me know. 😉

@hugosenari
Copy link
Author

hugosenari commented Feb 4, 2022

I tried similar approach, but it behaves different from what I would expect (return 404) 🤷

request = scan.call(_ = $body)

Have you ever considered trying 'aws way' (use some proprietary json instead of OpenApi)?

@hugosenari
Copy link
Author

I changed my local copy of atoz

method atozHook(call: OpenApiRestCall; url: Uri; input: JsonNode; body = ""): Recallable {.
    base.} =
  ## the hook is a terrible earworm
  var
    headers = newHttpHeaders(massageHeaders(input.getOrDefault("header")))
    text = body
  if text.len == 0 and "body" in input:
-    text = input.getOrDefault("body").getStr
+    text = $input.getOrDefault("body")
    if not headers.hasKey("content-type"):
      headers["content-type"] = "application/x-amz-json-1.0"
  else:
    headers["content-md5"] = base64.encode text.toMD5
  if not headers.hasKey($SecurityToken):
    let session = getEnv("AWS_SESSION_TOKEN", "")
    if session != "":
      headers[$SecurityToken] = session
  result = newRecallable(call, url, headers, text)
  result.atozSign(input.getOrDefault("query"), SHA256)

It worked, I'm not sure why we should expect body to be a jString with body instead of be JsonNode of body 🤔

Maybe

-    text = input.getOrDefault("body").getStr
+    if input.getOrDefault("body").kind == JString:
+      text = input.getOrDefault("body").getStr
+    else:
+      text = $input.getOrDefault("body")

@disruptek
Copy link
Owner

The problem is

assert $(JString"foo") != getStr(JString"foo") # foo versus "foo"

We have some code that does this same dance elsewhere; if we receive a JString we'll just render the string value, else we'll render true JSON. It kinda sucks and the proper solution is probably to use the hooks in openapi to fiddle with the values during validation or something.

I'm looking at the generator and what I'm using for my codegen at work is slightly different, so maybe it's time for me to just put the generator into the repo and remove the artifacts. It's already a problem supporting git-lfs for some reason.

And, FWIW, I don't mind replacing atoz with something else, but I don't use Nim anymore, so motivation is low. However, I don't mind scheming on how to go about it, so to speak, because it looks like I need to build a new atoz for Racket, so... 😉

@disruptek
Copy link
Owner

Okay, this https://github.com/disruptek/atoz/releases/tag/2626.4.0 has the generator in it so you can mess around more easily.

@hugosenari
Copy link
Author

Sorry I didn't have time to play with it yet, I'll just leave my thoughts here to not forget or help someone else (maybe you with Racket implementation).

assert $(JString"foo") != getStr(JString"foo") # foo versus "foo"

You're correct, I was thinking about the symptoms, not the problems.

General rule:

  1. if OpenAPI file say application/json it should be expecting $(JString"foo")
  2. else if OpenAPI file say text/plain for getStr(JString"foo")

Possible issues:

  1. Code isn't respecting content-type of OpenAPI file, is a generator issue.
  2. If OpenAPI file say application/json but server expects text/plain, bug is in OpenAPI file.
  3. if OpenAPI file say application/json and server expects application/json but with getStr(JString"foo"), is a server issue

We don't own OpenAPI file, but with appropriate information, I could register an issue there.

@disruptek
Copy link
Owner

disruptek commented Feb 8, 2022

I doubt my openapi code respects/sniffs (or even surfaces) the MIME type, so that's clearly a problem.

Basically, I hated Nim's httpclient so much that I didn't want to write the whole request/response loop using it; I assumed it would be replaced Soon so I delayed that effort. The predictable result, of course, was that I ended up using it for years with a hodge-podge of lame hacks. 🤦

As far as this problem goes, my openapi stuff was written against OpenAPI 2.x and while there's a PR to upgrade it to support OpenAPI 3.x, that work seems to've stalled.

The AWS APIs exported at https://github.com/APIs-guru/openapi-directory/tree/main/APIs/amazonaws.com don't appear to've been updated in awhile, but in any event, they are now OpenAPI 3-based.

@hugosenari
Copy link
Author

Random information for different approach:
AWS seems to be working on Smithy, that can read 'AWS API definitions' (JSON) and Smithy DSL for API definitions.

I couldn't find any .smithy definitions of AWS API, only JSON 🤷
if APIs-guru decease, Smithy has a tool for OpenAPI generation
There is an AWS SDK for rust with smithy, but is written in Kotlin

@disruptek
Copy link
Owner

Well, I know a few languages are doing this automagically using the Go API assets, including Clojure and Elixir; https://github.com/aws-beam/aws-elixir recently started automatically pushing codegen into trunk.

I thought Smithy was just what they were going to use in API Gateway but obviously that would be preferable for semantic reasons, if the AWS APIs are actually published in Smithy, I mean.

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