Skip to content

Commit

Permalink
fix!: use MultihashDigest type in stores (#1474)
Browse files Browse the repository at this point in the history
This forces callers to decode `Uint8Arrays` that may come from user
input before the storage method is called and that means we don't just
use them without validation that they are actually a multihash.

BREAKING CHANGE: `AllocationsStorage` and `BlobsStorage` methods not
take `MultihashDigest` types instead of `Uint8Array`s.
  • Loading branch information
Alan Shaw authored Jun 7, 2024
1 parent 237b0c6 commit 6c6a3bd
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 72 deletions.
8 changes: 3 additions & 5 deletions packages/upload-api/src/blob/accept.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ export function blobAcceptProvider(context) {
}

const { blob, space } = capability.nb
const digest = Digest.decode(blob.digest)
// If blob is not stored, we must fail
const hasBlob = await context.blobsStorage.has(blob.digest)
const hasBlob = await context.blobsStorage.has(digest)
if (hasBlob.error) {
return hasBlob
} else if (!hasBlob.ok) {
Expand All @@ -37,10 +38,7 @@ export function blobAcceptProvider(context) {
}
}

const digest = Digest.decode(blob.digest)
const createUrl = await context.blobsStorage.createDownloadUrl(
digest.bytes
)
const createUrl = await context.blobsStorage.createDownloadUrl(digest)
if (createUrl.error) {
return createUrl
}
Expand Down
8 changes: 5 additions & 3 deletions packages/upload-api/src/blob/allocate.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Server from '@ucanto/server'
import * as W3sBlob from '@web3-storage/capabilities/web3.storage/blob'
import * as Digest from 'multiformats/hashes/digest'
import * as API from '../types.js'
import {
BlobSizeOutsideOfSupportedRange,
Expand All @@ -26,6 +27,7 @@ export const allocate = async (context, { capability }) => {
}

const { blob, cause, space } = capability.nb
const digest = Digest.decode(blob.digest)
let { size } = blob

// We check if space has storage provider associated. If it does not
Expand All @@ -51,7 +53,7 @@ export const allocate = async (context, { capability }) => {
// While blob may exceed current maxUploadSize limit it could be that limit
// was higher in the past and user had this blob uploaded already in which
// case we should not error.
const exists = await context.allocationsStorage.exists(space, blob.digest)
const exists = await context.allocationsStorage.exists(space, digest)
if (exists.ok) {
return { ok: { size: 0 } }
} else {
Expand Down Expand Up @@ -88,7 +90,7 @@ export const allocate = async (context, { capability }) => {
// Check if we already have blob stored
// TODO: this may depend on the region we want to allocate and will need
// changes in the future.
const hasBlobStore = await context.blobsStorage.has(blob.digest)
const hasBlobStore = await context.blobsStorage.has(digest)
if (hasBlobStore.error) {
return hasBlobStore
}
Expand All @@ -106,7 +108,7 @@ export const allocate = async (context, { capability }) => {
const expiresIn = 60 * 60 * 24 // 1 day
const expiresAt = new Date(Date.now() + expiresIn).toISOString()
const createUploadUrl = await context.blobsStorage.createUploadUrl(
blob.digest,
digest,
blob.size,
expiresIn
)
Expand Down
6 changes: 3 additions & 3 deletions packages/upload-api/src/blob/get.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import * as Server from '@ucanto/server'
import * as Blob from '@web3-storage/capabilities/blob'
import * as Digest from 'multiformats/hashes/digest'
import * as API from '../types.js'
import { BlobNotFound } from './lib.js'
import { decode } from 'multiformats/hashes/digest'

/**
* @param {API.BlobServiceContext} context
* @returns {API.ServiceMethod<API.BlobGet, API.BlobGetSuccess, API.BlobGetFailure>}
*/
export function blobGetProvider(context) {
return Server.provide(Blob.get, async ({ capability }) => {
const { digest } = capability.nb
const digest = Digest.decode(capability.nb.digest)
const space = Server.DID.parse(capability.with).did()
const res = await context.allocationsStorage.get(space, digest)
if (res.error && res.error.name === 'RecordNotFound') {
return Server.error(new BlobNotFound(decode(digest)))
return Server.error(new BlobNotFound(digest))
}
return res
})
Expand Down
4 changes: 3 additions & 1 deletion packages/upload-api/src/blob/remove.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Server from '@ucanto/server'
import * as Blob from '@web3-storage/capabilities/blob'
import * as Digest from 'multiformats/hashes/digest'
import * as API from '../types.js'

import { RecordNotFoundErrorName } from '../errors.js'
Expand All @@ -11,7 +12,8 @@ import { RecordNotFoundErrorName } from '../errors.js'
export function blobRemoveProvider(context) {
return Server.provide(Blob.remove, async ({ capability }) => {
const space = capability.with
const { digest } = capability.nb
const digest = Digest.decode(capability.nb.digest)

const res = await context.allocationsStorage.remove(space, digest)
if (res.error && res.error.name === RecordNotFoundErrorName) {
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/upload-api/src/index/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const add = async ({ capability }, context) => {
* @returns {Promise<API.Result<API.Unit, API.IndexNotFound|API.ShardNotFound|API.SliceNotFound|API.Failure>>}
*/
const assertAllocated = async (context, space, digest, errorName) => {
const result = await context.allocationsStorage.exists(space, digest.bytes)
const result = await context.allocationsStorage.exists(space, digest)
if (result.error) return result
if (!result.ok)
return error(
Expand Down
13 changes: 7 additions & 6 deletions packages/upload-api/src/types/blob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
BlobRemoveSuccess,
BlobGetSuccess,
} from '@web3-storage/capabilities/types'
import { MultihashDigest } from 'multiformats'

import { RecordKeyConflict, ListResponse } from '../types.js'
import { Storage } from './storage.js'
Expand All @@ -21,11 +22,11 @@ export type TasksStorage = Storage<UnknownLink, Invocation>
export interface AllocationsStorage {
get: (
space: DID,
blobMultihash: Multihash
digest: MultihashDigest
) => Promise<Result<BlobGetSuccess, Failure>>
exists: (
space: DID,
blobMultihash: Multihash
digest: MultihashDigest
) => Promise<Result<boolean, Failure>>
/** Inserts an item in the table if it does not already exist. */
insert: (
Expand All @@ -38,7 +39,7 @@ export interface AllocationsStorage {
/** Removes an item from the table, returning zero on size if non existent. */
remove: (
space: DID,
digest: Multihash
digest: MultihashDigest
) => Promise<Result<BlobRemoveSuccess, Failure>>
}

Expand All @@ -61,9 +62,9 @@ export interface BlobAddInput {
export interface BlobAddOutput extends Omit<BlobAddInput, 'space' | 'cause'> {}

export interface BlobsStorage {
has: (content: Multihash) => Promise<Result<boolean, Failure>>
has: (content: MultihashDigest) => Promise<Result<boolean, Failure>>
createUploadUrl: (
content: Multihash,
content: MultihashDigest,
size: number,
/**
* The number of seconds before the presigned URL expires
Expand All @@ -81,5 +82,5 @@ export interface BlobsStorage {
Failure
>
>
createDownloadUrl: (content: Multihash) => Promise<Result<URI, Failure>>
createDownloadUrl: (content: MultihashDigest) => Promise<Result<URI, Failure>>
}
2 changes: 1 addition & 1 deletion packages/upload-api/test/handlers/web3.storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ export const test = {
assert.equal(locations.length, 1)

const loc = Result.unwrap(
await context.blobsStorage.createDownloadUrl(digest)
await context.blobsStorage.createDownloadUrl(multihash)
)
assert.ok(locations.includes(loc))
},
Expand Down
19 changes: 12 additions & 7 deletions packages/upload-api/test/storage/allocations-storage-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const test = {
const digest = multihash.bytes
const size = data.byteLength

const allocationGet0 = await allocationsStorage.get(spaceDid, digest)
const allocationGet0 = await allocationsStorage.get(spaceDid, multihash)
assert.ok(allocationGet0.error)
assert.equal(allocationGet0.error?.name, RecordNotFoundErrorName)

Expand Down Expand Up @@ -132,7 +132,7 @@ export const test = {
assert.ok(allocationInsert.ok)
assert.ok(allocationInsert.ok?.blob)

const allocationGet1 = await allocationsStorage.get(spaceDid, digest)
const allocationGet1 = await allocationsStorage.get(spaceDid, multihash)
assert.ok(allocationGet1.ok)
assert.ok(allocationGet1.ok?.blob)
assert.equal(allocationGet1.ok?.blob.size, size)
Expand All @@ -150,7 +150,10 @@ export const test = {
const digest = multihash.bytes
const size = data.byteLength

const allocationExist0 = await allocationsStorage.exists(spaceDid, digest)
const allocationExist0 = await allocationsStorage.exists(
spaceDid,
multihash
)
assert.ok(!allocationExist0.error)
assert.ok(!allocationExist0.ok)

Expand Down Expand Up @@ -180,7 +183,10 @@ export const test = {
assert.ok(allocationInsert.ok)
assert.ok(allocationInsert.ok?.blob)

const allocationExist1 = await allocationsStorage.exists(spaceDid, digest)
const allocationExist1 = await allocationsStorage.exists(
spaceDid,
multihash
)
assert.ok(allocationExist1.ok)
assert.ok(!allocationExist1.error)
},
Expand Down Expand Up @@ -315,9 +321,8 @@ export const test = {

const data = new Uint8Array([11, 22, 34, 44, 55])
const multihash = await sha256.digest(data)
const digest = multihash.bytes

const removeResult = await allocationsStorage.remove(spaceDid, digest)
const removeResult = await allocationsStorage.remove(spaceDid, multihash)

assert.ok(removeResult.error)
assert.equal(removeResult.error?.name, RecordNotFoundErrorName)
Expand Down Expand Up @@ -355,7 +360,7 @@ export const test = {
})
assert.ok(allocationInsert0.ok)

const removeResult = await allocationsStorage.remove(spaceDid, digest)
const removeResult = await allocationsStorage.remove(spaceDid, multihash)
assert.ok(removeResult.ok)
assert.equal(removeResult.ok?.size, size)
},
Expand Down
18 changes: 9 additions & 9 deletions packages/upload-api/test/storage/allocations-storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ export class AllocationsStorage {

/**
* @param {Types.DID} space
* @param {Uint8Array} blobMultihash
* @param {Types.MultihashDigest} digest
* @returns {ReturnType<Types.AllocationsStorage['get']>}
*/
async get(space, blobMultihash) {
async get(space, digest) {
const item = this.items.find(
(i) => i.space === space && equals(i.blob.digest, blobMultihash)
(i) => i.space === space && equals(i.blob.digest, digest.bytes)
)
if (!item) {
return { error: new RecordNotFound() }
Expand All @@ -51,24 +51,24 @@ export class AllocationsStorage {

/**
* @param {Types.DID} space
* @param {Uint8Array} blobMultihash
* @param {Types.MultihashDigest} digest
* @returns {ReturnType<Types.AllocationsStorage['exists']>}
*/
async exists(space, blobMultihash) {
async exists(space, digest) {
const item = this.items.find(
(i) => i.space === space && equals(i.blob.digest, blobMultihash)
(i) => i.space === space && equals(i.blob.digest, digest.bytes)
)
return { ok: !!item }
}

/**
* @param {Types.DID} space
* @param {Uint8Array} blobMultihash
* @param {Types.MultihashDigest} digest
* @returns {ReturnType<Types.AllocationsStorage['remove']>}
*/
async remove(space, blobMultihash) {
async remove(space, digest) {
const item = this.items.find(
(i) => i.space === space && equals(i.blob.digest, blobMultihash)
(i) => i.space === space && equals(i.blob.digest, digest.bytes)
)
if (!item) {
return { error: new RecordNotFound() }
Expand Down
8 changes: 4 additions & 4 deletions packages/upload-api/test/storage/blobs-storage-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const test = {
size: size,
}
const createUploadUrl = await blobsStorage.createUploadUrl(
blob.digest,
multihash0,
blob.size,
expiresIn
)
Expand All @@ -41,7 +41,7 @@ export const test = {
assert.equal(goodPut.status, 200, await goodPut.text())

// check it exists
const hasBlob = await blobsStorage.has(blob.digest)
const hasBlob = await blobsStorage.has(multihash0)
assert.ok(hasBlob.ok)
},

Expand All @@ -54,7 +54,7 @@ export const test = {
const expires = 60 * 60 * 24 // 1 day

const upload = Result.unwrap(
await blobsStorage.createUploadUrl(digest.bytes, data.length, expires)
await blobsStorage.createUploadUrl(digest, data.length, expires)
)

await fetch(upload.url, {
Expand All @@ -65,7 +65,7 @@ export const test = {
})

const downloadUrl = Result.unwrap(
await blobsStorage.createDownloadUrl(digest.bytes)
await blobsStorage.createDownloadUrl(digest)
)

const res = await fetch(downloadUrl)
Expand Down
Loading

0 comments on commit 6c6a3bd

Please sign in to comment.