ZA̡͊͠͝LGΌ causes "Invalid MD5 checksum on messages"

https://github.com/aws/aws-sdk-js-v3/issues/2362

by paulddraper • 5 years ago

178 68 5 years ago

ec109685• 5 years ago

That pull request needs tests.

ninkendo2• 5 years ago

I love how it just blithely takes on another dependency too. This is all too common in JS dev unfortunately… never implement something yourself if you can take on another useless dependency instead.

pindab0ter• 5 years ago

Conversely, you don’t have to reinvent the wheel of parsing XML and all the nuances that come with that if you use a tried and tested library.

I see your point and agree with the general sentiment, but this is no leftpad.

mdaniel2• 5 years ago

Strictly speaking "he@1.2.0" was already present, drug in transitively by something else, that's why the yarn.lock folded the older version into the existing checksum

I came very close to commenting on the PR, but since it was pointed out elsewhere that the fix was to the wrong file, I suspected it was going to be closed wontfix anyway

paulddraper7• 5 years ago

never implement something yourself if you can take on another useless dependency instead.

So...your argument is that XML parsing/decoding is useless/too easy and every project should reimplement an XML parser/decoder?

If so, I have 0% trust in your judgement.

cldellow• 5 years ago

The node SDKs for AWS really shock me.

The `aws-sdk` v2 API is huge. It and its transitive deps are 64MB. In one TS project that I worked on, adding it added 1.5 seconds of transpilation time.

I thought v3 was supposed to be a modular improvement, but even just the `@aws-sdk/client-s3` library is 24MB.

Then I see bugs like `v3 has a dependency on react-native` and I really wonder how the process works internally to release these things.

lvh• 5 years ago

The Java SDKs are bigger, and what's IMO worse: in Lambda the Python and JavaScript SDKs are automatically installed but the Java SDK is not automagically available on the classpath. Since Lambda is an environment where both artifact size and startup latency matters a lot, this kind of hobbles Java Lambdas.

(I work around this mostly by using native-image.)

uo21tp5hoyg• 5 years ago

This problem is what lead me to building out my own s3-compatible implementation, sure it doesn't have all the bells and whistles that the official library has but it does what I need and is under 10kb in size.

paulddraper7• 5 years ago

As I found out in this bug, v3, despite being released last year, is unusable.

You can't even stream an object to S3. https://github.com/aws/aws-sdk-js-v3/issues/2348

renke1• 5 years ago

I was pretty shocked by the bad quality of the Cognito JS library (which is now superseded by the Amplify JS library, afaik). The API design itself was really bad and error handling was based on parsing the textual messages of the HTTP responses (or at least something along those lines). In general I really do like AWS but it made me reluctant to suggest Congito to any of our clients (as opposed to say Firebase Auth which has a nice API and just works).

dragonwriter3• 5 years ago

I was pretty shocked by the bad quality of the Cognito JS library (which is now superseded by the Amplify JS library, afaik).

Subsumed within rather than superceded, AFAICT—in fact, I’ve had to use notionally outdated documentation (actually, I think it was actually Stack Overflow answers, but...) for the former to fill gaps in that for the latter, so I’m relatively certain of the relationship.

AWS SDK APIs tend to mix poor design with poor documentation, often being extremely leaky abstractions on top of the HTTP APIs, which themselves aren’t masterpieces of either design or documentation.

chx• 5 years ago

The PHP SDK is so bad, https://github.com/async-aws/ exists and one of the stated goals is "One should be able to read the code and follow the logic". https://bref.sh/ which allows using PHP to write AWS Lambda functions easily uses async-aws, for example.

foxbarrington• 5 years ago

For anyone who doesn't get the reference: https://stackoverflow.com/questions/1732348/regex-match-open...

TacticalCoder2• 5 years ago

Oh you beat me to it.

Rendello• 5 years ago

regex will consume all living tissue (except for HTML which it cannot, as previously prophesied)

This part gets me every time!

isoprophlex• 5 years ago

The <center> cannot hold it is too late.

Never fails to make me chuckle

jonathanoliver• 5 years ago

Where I work, we have this post as part of our software developer on-boarding experience. It's required reading.

goto115• 5 years ago

Have any of the developers noticed the fundamental fallacy in the answer? The question is not about parsing, it is about recognizing a token in a string, which is exactly what regular expressions are designed for.

Dylan168072• 5 years ago

Which is a fun reference but this isn't exactly parsing and I think using one regex to process escapes would actually fix the problem.

Edit: The function in the fix does in fact appear to be a straightforward implementation based around a regex. https://github.com/mathiasbynens/he/blob/master/src/he.js

yarcob2• 5 years ago

I think the Zalgo answer is an wonderful example of people thinking they are smarter than they really are.

Spot the error:

1) HTML is not a regular language

2) Regular expressions can only parse regular languages

3) Regular expressions can not be used to to parse arbitrary HTML documents

4) If you try to use regular expressions to parse HTML documents, you are doing it wrong

The error is between step 3 and 4: It's true that you can't write a regex that parses arbitrary HTML.

However, very often we do not have to deal with arbitrary HTML. Very often we have to deal with documents that only use a subset of HTML and they can be parsed by regular expressions just fine.

paulddraper7• 5 years ago

a straightforward implementation based around a regex

A "straightforward regex" is so large/complicated the solution actually uses a JS code generator.

Yes, decoding is only part of XML parsing, and you technically can use a regex for the coding.

But 9/10 times sometimes rolls a regex to help parse XML they fail miserably. Including AWS engineers.

goto115• 5 years ago

A rebuttal here: https://www.cargocultcode.com/solving-the-zalgo-regex/

sobellian• 5 years ago

I somewhat disagree that the famous answer is an instance of cargo-culting. Even if not for entertainment it's probably more valuable than literally answering the question posed. Let's say that you now have a method that extracts tags in the manner prescribed... now what? I'd bet the OP was only trying to write that method as a subprocedure for some larger task much more efficiently dealt with another way. In fact if you look at that poster's question history around that time, it's apparent that they were trying to render potentially-malformed XHTML.

mintplant• 5 years ago

Worth mentioning that Zalgo predates the StackOverflow post (a̷nd̡ t̕h͟i̡s u̕niver͠s͠e̵).

https://en.wikipedia.org/wiki/Zalgo_text

TacticalCoder2• 5 years ago

For those unaware of that folklore, the github link contains reference to one of the most famous StackOverflow answer of all times:

https://stackoverflow.com/questions/1732348/regex-match-open...

pcthrowaway• 5 years ago

Thanks for sharing, I hadn't seen that. Sadly, I feel like it would get removed as an answer or edited to the SO standards if that was posted as an answer today.

paulddraper7• 5 years ago

Unquestionably.

SO's sense of humor died around 2014.

mattmanser3• 5 years ago

The offending code for those interested:

   const decodeEscapedXML = (str: string) =>
      str
        .replace(/&amp;/g, "&")
        .replace(/&apos;/g, "'")
        .replace(/&quot;/g, '"')
        .replace(/&gt;/g, ">")
        .replace(/&lt;/g, "<");

Seems like it's in multiple places in the code base too, I think all those clients are automatically generated.

https://github.com/aws/aws-sdk-js-v3/search?p=1&q=const+deco...

So that's not even a good fix, think he should have fixed

aws-sdk-js-v3/codegen/smithy-aws-typescript-codegen/src/main/resources/software/amazon/smithy/aws/typescript/codegen/decodeEscapedXML.ts

formerly_proven• 5 years ago

That's dreadful code.

bartwe• 5 years ago

Yeah and unpacking & first is asking for trouble with say "&amp;lt;" in your input

0x000000E2• 5 years ago

My god. I wish I could find a linter to ban startsWith and regex from my codebases. I swear 99% of their uses are buggy hacky shit

chrisandchris• 5 years ago

Can‘t tell you how often I have to use StartsWith/Contains in some interface when connecting to applications together because of the missing ability to extract proper unique/primary keys for objects from any of the systems.

So please don‘t, I need these buggy hacks ;)

mdaniel2• 5 years ago

You may find semgrep handy for that, although I don't offhand know how to express "any use of regex" in its language

https://github.com/returntocorp/semgrep

paulddraper7• 5 years ago

Very true. AWS doesn't pay me enough to spend time on a comprehensive fix, with tests.

Hopefully they will pay someone to do that.

At least I can read SQS messages now.

ufmace• 5 years ago

I've become a lot more sympathetic to this attitude lately.

I kind of enjoy working with Rust, and have written a few minor utilities to interact with AWS in it for work. I found and fixed a few bugs in the library for it, Rusoto. I observed along the way that the project is sorely in need of people to spend time on management and maintenance. I could do that, but...

I can't ignore that AWS makes some absurdly huge amount of money for Amazon. I don't begrudge them that, but I get paid pretty well at my day job already. Why do more of that work for the benefit of AWS for no money? They ought to pay me for it.

Hell, they have more than enough money to hire a team of professional experts in every language under the sun to maintain their AWS libs. Especially considering that the net effect of higher-quality AWS libs in more languages will result in more money being spent on AWS services and more lock-in. There's no excuse for having such terrible code in a mainstream language like Javascript.

mattmanser3• 5 years ago

Yeah, but you've submitted a PR. It's not a fix, it's broken code. As soon as they run their code generator, it'll disappear.

raesene9• 5 years ago

(possibly) interestingly, I pased the link to this issue into a slack, which very helpfullly got one of Github's new preview links.

The image it produces can't render Zalgo either https://imgur.com/a/pOUu7m1

wereHamster• 5 years ago

lol no even a single test in the PR which supposedly fixes this.

0xbadcafebee• 5 years ago

Oh hey, another person making a pipeline for EHR records. I wonder how many of these have been remade by now.

tyingq• 5 years ago

"Thanks to (1), all my HL7 messages in my SQS FIFO queue won't process because carriage returns broke the client."

Did my time with HL7. One of those cases where a standard didn't serve any real purpose. It gave the appearance that it might work, but then bred a whole industry of HL7 <-> HL7 translators because nobody's HL7 is the same. If you need a translator/hub, everyone might as well have their own protocol.

paulddraper7• 5 years ago

Yeah.

I also work with X12 which is far more consistent (because if it isn't, you don't get paid).

But the format itself is -- believe it or not -- even worse.

ZA̡͊͠͝LGΌ causes "Invalid MD5 checksum on messages"

Related Stories