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

fix!: pss target length verification #384

Merged
merged 5 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ export class Bee {
*
* @param postageBatchId Postage BatchId that will be assigned to sent message
* @param topic Topic name
* @param target Target message address prefix
* @param target Target message address prefix. Has a limit on length. Recommend to use `Utils.Pss.makeMaxTarget()` to get the most specific target that Bee node will accept.
* @param data Message to be sent
* @param recipient Recipient public key
* @throws TypeError if `data`, `batchId`, `target` or `recipient` are in invalid format
Expand Down
2 changes: 1 addition & 1 deletion src/modules/pss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function send(
): Promise<void> {
await http<BeeGenericResponse>(ky, {
method: 'post',
path: `${endpoint}/send/${topic}/${target.slice(0, 4)}`,
path: `${endpoint}/send/${topic}/${target}`,
body: await prepareData(data),
responseType: 'json',
searchParams: { recipient },
Expand Down
1 change: 1 addition & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface Dictionary<T> {
}

export const ADDRESS_HEX_LENGTH = 64
export const PSS_TARGET_HEX_LENGTH_MAX = 4
export const PUBKEY_HEX_LENGTH = 66
export const BATCH_ID_HEX_LENGTH = 64
export const REFERENCE_HEX_LENGTH = 64
Expand Down
1 change: 1 addition & 0 deletions src/utils/expose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export * as Tar from './tar'
export * as Eth from './eth'
export * as Stream from './stream'
export { keccak256Hash } from './hash'
export { makeMaxTarget } from './pss'
16 changes: 16 additions & 0 deletions src/utils/pss.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { AddressPrefix, PSS_TARGET_HEX_LENGTH_MAX } from '../types'

/**
* Utility function that for given strings/reference takes the most specific
* target that Bee node will except.
Copy link
Member

Choose a reason for hiding this comment

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

does not the bee accept any length of target? then why we allow to pass any length address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. You might got this impression from Bee-js usage of PSS before this patch, which silently sliced the target to max. allowed length without user's knowledge. This patch impose correct validation and tools for the user to have predictable behavior.

*
* @param target is a non-prefixed hex string Bee address
* @see [Bee docs - PSS](https://docs.ethswarm.org/docs/dapps-on-swarm/pss)
*/
export function makeMaxTarget(target: string): AddressPrefix {
AuHau marked this conversation as resolved.
Show resolved Hide resolved
if (typeof target !== 'string') {
throw new TypeError('target has to be an string!')
}

return target.slice(0, PSS_TARGET_HEX_LENGTH_MAX)
}
5 changes: 3 additions & 2 deletions src/utils/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
Tag,
TAGS_LIMIT_MAX,
TAGS_LIMIT_MIN,
PSS_TARGET_HEX_LENGTH_MAX,
UploadOptions,
TransactionHash,
} from '../types'
Expand Down Expand Up @@ -191,9 +192,9 @@ export function assertTag(value: unknown): asserts value is Tag {
export function assertAddressPrefix(value: unknown): asserts value is AddressPrefix {
assertHexString(value, undefined, 'AddressPrefix')

if (value.length > ADDRESS_HEX_LENGTH) {
if (value.length > PSS_TARGET_HEX_LENGTH_MAX) {
throw new BeeArgumentError(
`AddressPrefix must have length of ${ADDRESS_HEX_LENGTH} at most! Got string with ${value.length}`,
`AddressPrefix must have length of ${PSS_TARGET_HEX_LENGTH_MAX} at most! Got string with ${value.length}`,
value,
)
}
Expand Down
10 changes: 8 additions & 2 deletions test/integration/bee-class.browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ describe('Bee class - in browser', () => {
const beePeer = new window.BeeJs.Bee(BEE_PEER_URL)

const receive = bee.pssReceive(topic)
await beePeer.pssSend(batchIdPeer, topic, overlay, message)
await beePeer.pssSend(batchIdPeer, topic, window.BeeJs.Utils.makeMaxTarget(overlay), message)

const msg = await receive

Expand Down Expand Up @@ -242,7 +242,13 @@ describe('Bee class - in browser', () => {
const beePeer = new window.BeeJs.Bee(BEE_PEER_URL)

const receive = bee.pssReceive(topic)
await beePeer.pssSend(batchIdPeer, topic, overlay, message, pssPublicKey)
await beePeer.pssSend(
batchIdPeer,
topic,
window.BeeJs.Utils.makeMaxTarget(overlay),
message,
pssPublicKey,
)

const msg = await receive

Expand Down
26 changes: 19 additions & 7 deletions test/integration/bee-class.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Bee, BeeArgumentError, BeeDebug, Collection } from '../../src'
import { Bee, BeeArgumentError, BeeDebug, Collection, PssSubscription } from '../../src'
import { makeSigner } from '../../src/chunk/signer'
import { makeSOCAddress, uploadSingleOwnerChunkData } from '../../src/chunk/soc'
import { ChunkReference } from '../../src/feed'
Expand Down Expand Up @@ -30,6 +30,7 @@ import {
} from '../utils'
import { Readable } from 'stream'
import { TextEncoder } from 'util'
import { makeMaxTarget } from '../../src/utils/pss'

commonMatchers()

Expand Down Expand Up @@ -300,7 +301,7 @@ describe('Bee class', () => {
})

const { overlay } = await beeDebug.getNodeAddresses()
await beePeer.pssSend(getPostageBatch(BEE_DEBUG_PEER_URL), topic, overlay, message)
await beePeer.pssSend(getPostageBatch(BEE_DEBUG_PEER_URL), topic, makeMaxTarget(overlay), message)
})().catch(reject)
})
},
Expand All @@ -323,7 +324,13 @@ describe('Bee class', () => {
})

const { overlay, pssPublicKey } = await beeDebug.getNodeAddresses()
await beePeer.pssSend(getPostageBatch(BEE_DEBUG_PEER_URL), topic, overlay, message, pssPublicKey)
await beePeer.pssSend(
getPostageBatch(BEE_DEBUG_PEER_URL),
topic,
makeMaxTarget(overlay),
message,
pssPublicKey,
)
})().catch(reject)
})
},
Expand All @@ -334,15 +341,16 @@ describe('Bee class', () => {
'should subscribe to topic',
async () => {
return new Promise<void>((resolve, reject) => {
let subscription: PssSubscription
;(async () => {
const topic = 'bee-class-subscribe-topic'
const message = new Uint8Array([1, 2, 3])
const beeDebug = new BeeDebug(beeDebugUrl())

const subscription = bee.pssSubscribe(topic, {
subscription = bee.pssSubscribe(topic, {
onMessage: receivedMessage => {
// without cancel jest complains for leaking handles and may hang
subscription.cancel()
subscription?.cancel()

expect(receivedMessage).toEqual(message)
resolve()
Expand All @@ -353,8 +361,12 @@ describe('Bee class', () => {
})

const { overlay } = await beeDebug.getNodeAddresses()
await beePeer.pssSend(getPostageBatch(BEE_DEBUG_PEER_URL), topic, overlay, message)
})().catch(reject)
await beePeer.pssSend(getPostageBatch(BEE_DEBUG_PEER_URL), topic, makeMaxTarget(overlay), message)
})().catch(e => {
// without cancel jest complains for leaking handles and may hang
subscription?.cancel()
reject(e)
})
})
},
PSS_TIMEOUT,
Expand Down
14 changes: 11 additions & 3 deletions test/integration/modules/pss.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as pss from '../../../src/modules/pss'
import * as connectivity from '../../../src/modules/debug/connectivity'
import { beeDebugKy, beeKy, beePeerDebugUrl, beePeerKy, beeUrl, getPostageBatch, PSS_TIMEOUT } from '../../utils'
import { makeMaxTarget } from '../../../src/utils/pss'

const BEE_KY = beeKy()
const BEE_URL = beeUrl()
Expand All @@ -20,7 +21,7 @@ describe('modules/pss', () => {
expect(peers.length).toBeGreaterThan(0)

const target = peers[0].address
await pss.send(BEE_KY, topic, target, message, getPostageBatch()) // Nothing is asserted as nothing is returned, will throw error if something is wrong
await pss.send(BEE_KY, topic, makeMaxTarget(target), message, getPostageBatch()) // Nothing is asserted as nothing is returned, will throw error if something is wrong
},
PSS_TIMEOUT,
)
Expand Down Expand Up @@ -48,7 +49,7 @@ describe('modules/pss', () => {

const addresses = await connectivity.getNodeAddresses(BEE_DEBUG_KY)
const target = addresses.overlay
await pss.send(BEE_PEER_KY, topic, target, message, getPostageBatch(BEE_DEBUG_PEER_URL))
await pss.send(BEE_PEER_KY, topic, makeMaxTarget(target), message, getPostageBatch(BEE_DEBUG_PEER_URL))
})().catch(reject)
})
},
Expand Down Expand Up @@ -80,7 +81,14 @@ describe('modules/pss', () => {
const addresses = await connectivity.getNodeAddresses(BEE_DEBUG_KY)
const target = addresses.overlay
const recipient = addresses.pssPublicKey
await pss.send(BEE_PEER_KY, topic, target, message, getPostageBatch(BEE_DEBUG_PEER_URL), recipient)
await pss.send(
BEE_PEER_KY,
topic,
makeMaxTarget(target),
message,
getPostageBatch(BEE_DEBUG_PEER_URL),
recipient,
)
})().catch(reject)
})
},
Expand Down
12 changes: 3 additions & 9 deletions test/unit/assertions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,13 @@ export function testAddressPrefixAssertions(executor: (input: unknown) => void):
await expect(() => executor('')).rejects.toThrow(TypeError)

// Not an valid hexstring (ZZZ)
await expect(() => executor('ZZZfb5a872396d9693e5c9f9d7233cfa93f395c093371017ff44aa9ae6564cdd')).rejects.toThrow(
TypeError,
)
await expect(() => executor('ZZZf')).rejects.toThrow(TypeError)

// Prefixed hexstring is not accepted
await expect(() => executor('0x634fb5a872396d9693e5c9f9d7233cfa93f395c093371017ff44aa9ae6564cdd')).rejects.toThrow(
TypeError,
)
await expect(() => executor('0x634f')).rejects.toThrow(TypeError)

// Too long hexstring
await expect(() => executor('123634fb5a872396d9693e5c9f9d7233cfa93f395c093371017ff44aa9ae6564cdd')).rejects.toThrow(
BeeArgumentError,
)
await expect(() => executor('12364')).rejects.toThrow(BeeArgumentError)
})
}

Expand Down