Skip to content

Create digest-stream.md#4919

Closed
simonhaenisch wants to merge 1 commit into
cloudflare:productionfrom
simonhaenisch:patch-3
Closed

Create digest-stream.md#4919
simonhaenisch wants to merge 1 commit into
cloudflare:productionfrom
simonhaenisch:patch-3

Conversation

@simonhaenisch

Copy link
Copy Markdown

After dropping this as a comment here: w3c/webcrypto#73 (comment), I thought might be worth turning it into an example for the docs as well?

Feel free to update this (i've allowed edits by maintainers to the branch in my fork).

After dropping this as a comment here: w3c/webcrypto#73 (comment), I thought might be worth turning it into an example for the docs as well?

Feel free to update this.
@github-actions
github-actions Bot requested a review from deadlypants1973 June 24, 2022 12:30
@deadlypants1973 deadlypants1973 self-assigned this Jun 28, 2022
@deadlypants1973 deadlypants1973 added this to the Pages and Workers milestone Jun 28, 2022
@simonhaenisch

Copy link
Copy Markdown
Author

BTW crypto.DigestStream got announced here in December (found a link to that post via the Github issue), but there's nothing in the docs yet (neither mentioned in Using Streams nor Streams API Reference), so this is the working result of my trial-and-error 😅

@kristianfreeman
kristianfreeman self-requested a review July 7, 2022 12:50
@vlovich

vlovich commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

I would recommend using sha512 in the example with a comment that it's faster than sha1 (it is). It's also faster than sha256 for longer messages but slightly slower/the same for shorter ones.

processBody should have an implementation I think so that the example is copyable without doing anything.

I believe all our docs are currently JS unfortunately so we should probably remain consistent until we get UI elements to dynamically switch between languages.

Might not be a bad idea to return the ETag in the response header (don't forget to put quotes around it - etags in HTTP have double quotes).

const encoder = new TextEncoder();

const etagStream = new crypto.DigestStream('SHA-1');
const { readable, writable } = new TransformStream();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Better to use new IdentityTransformStream() here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry it's been too long, looking at this again now. Can you explain the difference, please?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code is using the TransformStream essentially as an identity passthrough. It is not actually transforming any bytes. This is the exact use case for IdentityTransformStream. Soon, the default implementation of new TransformStream() is going to change to be more spec compliant. It will still be an identity passthrough by default but the underlying implementation is a bit more complex. To maintain the very simple use case you have here, IdentityTransformStream is the right choice.

const { readable, writable } = new TransformStream();

const etagWriter = etagStream.getWriter();
const resultWriter = writable.getWriter();

@jasnell jasnell Jul 7, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An alternative approach here is to tee the streams.

const { writable, readable } = new IdentityTransformStream();
const [ branch1, branch2 ] = readable.tee();

env.KV.put('result', branch1);

async getDigest(readable) {
  const etagStream = new crypto.DigestStream('SHA-1');
  await readable.pipeTo(etagStream);
  return etagStream.digest;
}

const writer = writable.getWriter();
writer.write('hello');
writer.close();

const etag = await getDigest(branch2);
// ...

@gitguardian

gitguardian Bot commented Jul 16, 2022

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
- Generic High Entropy Secret 7d3d958 config.toml View secret
- Generic High Entropy Secret 58b6333 products/ddos-protection/docs-config.js View secret
- Generic High Entropy Secret f5ad72d products/ddos-protection/docs-config.js View secret
3543783 Generic High Entropy Secret 51cbbad products/ruleset-engine/docs-config.js View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@cloudflare cloudflare deleted a comment from Disneimabong Nov 2, 2022
@deadlypants1973

Copy link
Copy Markdown
Contributor

Hi @simonhaenisch , is this ready for merge? Could you please look through and accept or resolve James' suggestions?

@simonhaenisch

simonhaenisch commented Nov 9, 2022

Copy link
Copy Markdown
Author

Hey sorry haven't gotten around to it, so no not yet (let me convert it to draft PR). I'll try to get to it asap.

@simonhaenisch
simonhaenisch marked this pull request as draft November 9, 2022 17:10
@deadlypants1973

deadlypants1973 commented Jun 1, 2023

Copy link
Copy Markdown
Contributor

Hi @simonhaenisch , would you still like to work on this? Let me know, and thank you

@simonhaenisch

Copy link
Copy Markdown
Author

Hey sorry don't really have to time to finish it currently... can close it for now, and it would still be here as reference i guess.

@KimJ15

KimJ15 commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

Closing per the last comment - can reopen at a later date.

@KimJ15 KimJ15 closed this Jun 21, 2023
@Disneimabong

Disneimabong commented Jul 19, 2026 via email

Copy link
Copy Markdown

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

Successfully merging this pull request may close these issues.

6 participants