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.

Sign up for GitHub

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

Jump to bottom

Port fix for rejection of empty CAR files #355

Closed
alanshaw opened this issue Aug 31, 2021 · 2 comments · Fixed by #899
Closed

Port fix for rejection of empty CAR files #355

alanshaw opened this issue Aug 31, 2021 · 2 comments · Fixed by #899
Assignees
@alanshaw
Labels
effort/hours Estimated to take one or several hours kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@alanshaw
Copy link
Contributor

web3-storage/web3.storage#344
@alanshaw alanshaw added kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up need/triage Needs initial labeling and prioritization effort/hours Estimated to take one or several hours labels Aug 31, 2021
@alanshaw alanshaw self-assigned this Sep 14, 2021
@dchoi27 dchoi27 added P2 Medium: Good to have, but can wait until someone steps up and removed P1 High: Likely tackled by core team if no one steps up labels Sep 17, 2021
@alanshaw alanshaw mentioned this issue Oct 19, 2021
Merged
@atopal atopal removed the need/triage Needs initial labeling and prioritization label Oct 22, 2021
@JeffLowe
Copy link
Contributor

related to #390 per Alan

@dchoi27
Copy link
Contributor

dchoi27 commented Nov 12, 2021

Yeah more precisely #645 will close this

alanshaw pushed a commit that referenced this issue Nov 16, 2021
feat!: automatic client side CAR chunking for large data (#588)
437ae4f 
`storeBlob`, `storeDirectory` and `store` now construct CAR files and then call `storeCar` (which `POST`s to `/upload`). This automatically gives them CAR chunking capability.

It adds the following **static** functions:

```ts
NFTStorage.encodeBlob(blob: Blob): Promise<{ cid: CID, car: CarReader }>

NFTStorage.encodeDirectory(files: Blob[]): Promise<{ cid: CID, car: CarReader }>

NFTStorage.encodeNFT<T extends TokenInput>(input: T): Promise<{ cid: CID, token: Token<T>, car: CarReader }>
```

After encoding your CAR and obtaining the root CID you can call:

```js
await client.storeCar(car)
```

🚨 There are trade offs here:

1. We're always sending CAR files so our `type` field is going to always be `Car` for uploads from the JS client.

    To mitigate this we could simply inspect the root node of the CAR, from this we can assertain the type of the data. We should talk about our `type` field and what it means. Right now we've mapped it to the method of upload...lets resolve this and submit a separate PR to fix.

    FYI, I'm going to be implementing [#355](#355) soon so will be inspecting the root node of the CAR anyway for validation purposes.

2. The `files` property is not set for the `Multipart` or `Nft` type uploads (since it's being uploaded as a CAR).

    I actually can't see any requests to the `/status/:cid` API (literally 0 - everyone uses `/check/:cid`) in the cloudflare logs which is the only place this data is exposed to users. I don't think folks will miss it. However if we inspect the root node we can get a shallow directory listing to put here.

    For `Nft` types we can't really set it anymore. There's also a regression right now where we're _not_ setting it anyway. As an aside I'm not sure how much value it has since the data is just a list of file names, without paths within the object that is created...

resolves #220
@alanshaw alanshaw mentioned this issue Nov 29, 2021
Closed
@alanshaw alanshaw mentioned this issue Dec 6, 2021
Merged
alanshaw pushed a commit that referenced this issue Jan 21, 2022
feat: s3 backups (#899)
60f17c0 
This uploads data sent to the `/upload` endpoint to S3 for disaster recovery. All data is converted to CAR (if not CAR already) before being uploaded to S3.

Note: does not backup pinned data or IPNFT's uploaded to the `/store` endpoint

resolves #390
resolves #637
resolves #355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alanshaw alanshaw

Labels
effort/hours Estimated to take one or several hours kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: s3 backups nftstorage/nft.storage
feat: s3 backups nftstorage/nft.storage
4 participants
@alanshaw @atopal @JeffLowe @dchoi27