Send an integer exp for video upload service auth tokens
The legacy upload paths built the service auth expiry with `Date.now() / 1000 + 60 * 30`, which leaves the millisecond remainder in place and produces a fractional Unix timestamp. `com.atproto.server.getServiceAuth` types `exp` as an integer and serializes it straight into the query string, so the request fails validation and the upload dies before it starts. The multipart path already floored correctly, which is why only the legacy/fallback transports were affected. Rather than patch the two arithmetic expressions, add a shared `serviceAuthExp()` helper and route all three call sites through it, and floor inside `getServiceAuthToken` itself so a future call site that hand-rolls the arithmetic cannot reintroduce the bug. A non-finite exp now throws with a clear message instead of sending NaN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013T6CzrmaiVUkdWwi4FjnB2
This commit is contained in:
@@ -0,0 +1,70 @@
|
||||
import {type Client} from '@atproto/lex'
|
||||
|
||||
import {com} from '#/lexicons'
|
||||
import {
|
||||
getServiceAuthToken,
|
||||
SERVICE_AUTH_TTL_SEC,
|
||||
serviceAuthExp,
|
||||
} from '../upload.shared'
|
||||
|
||||
function createClient() {
|
||||
const call = jest.fn().mockResolvedValue({token: 'token'})
|
||||
return {client: {call} as unknown as Client, call}
|
||||
}
|
||||
|
||||
describe('serviceAuthExp', () => {
|
||||
it('returns an integer even when the clock has sub-second precision', () => {
|
||||
jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_500)
|
||||
expect(serviceAuthExp()).toBe(1_700_000_000 + SERVICE_AUTH_TTL_SEC)
|
||||
expect(Number.isInteger(serviceAuthExp())).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts a custom ttl and keeps the result integral', () => {
|
||||
jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_999)
|
||||
expect(serviceAuthExp(90.7)).toBe(1_700_000_090)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getServiceAuthToken', () => {
|
||||
it('floors a fractional exp before sending it', async () => {
|
||||
const {client, call} = createClient()
|
||||
await getServiceAuthToken({
|
||||
client,
|
||||
aud: 'did:web:video.bsky.app',
|
||||
lxm: 'com.atproto.repo.uploadBlob',
|
||||
exp: 1_700_001_800.5,
|
||||
})
|
||||
expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, {
|
||||
aud: 'did:web:video.bsky.app',
|
||||
lxm: 'com.atproto.repo.uploadBlob',
|
||||
exp: 1_700_001_800,
|
||||
})
|
||||
})
|
||||
|
||||
it('leaves exp undefined when the caller omits it', async () => {
|
||||
const {client, call} = createClient()
|
||||
await getServiceAuthToken({
|
||||
client,
|
||||
aud: 'did:web:video.bsky.app',
|
||||
lxm: 'app.bsky.video.getUploadLimits',
|
||||
})
|
||||
expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, {
|
||||
aud: 'did:web:video.bsky.app',
|
||||
lxm: 'app.bsky.video.getUploadLimits',
|
||||
exp: undefined,
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects a non-finite exp rather than sending NaN', async () => {
|
||||
const {client, call} = createClient()
|
||||
await expect(
|
||||
getServiceAuthToken({
|
||||
client,
|
||||
aud: 'did:web:video.bsky.app',
|
||||
lxm: 'com.atproto.repo.uploadBlob',
|
||||
exp: NaN,
|
||||
}),
|
||||
).rejects.toThrow('Invalid service auth exp')
|
||||
expect(call).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -5,7 +5,7 @@ import {AbortError} from '#/lib/async/cancelable'
|
||||
import {type CompressedVideo} from '#/lib/media/video/types'
|
||||
import {shouldRetryError} from '#/lib/strings/errors'
|
||||
import {type app} from '#/lexicons'
|
||||
import {getServiceAuthToken} from '../upload.shared'
|
||||
import {getServiceAuthToken, serviceAuthExp} from '../upload.shared'
|
||||
import {mimeToExt} from '../util'
|
||||
import {
|
||||
abortUpload,
|
||||
@@ -280,7 +280,7 @@ function createTokenProvider(
|
||||
async function get(forceRefresh = false) {
|
||||
if (!forceRefresh && token && Date.now() < expiresAt - 60_000) return token
|
||||
if (!refresh) {
|
||||
const exp = Math.floor(Date.now() / 1000) + 60 * 30
|
||||
const exp = serviceAuthExp()
|
||||
refresh = getServiceAuthTokenWithRetry(client, dispatchUrl, exp, signal)
|
||||
.then(nextToken => {
|
||||
token = nextToken
|
||||
|
||||
@@ -27,6 +27,11 @@ export async function getServiceAuthToken({
|
||||
dispatchUrl?: string | URL
|
||||
aud?: string
|
||||
lxm: NsidString
|
||||
/**
|
||||
* Unix timestamp in *seconds* at which the token expires. Fractional values
|
||||
* are floored - see {@link toIntegerExp}. Defaults to the server's own
|
||||
* short expiry when omitted.
|
||||
*/
|
||||
exp?: number
|
||||
}) {
|
||||
let resolvedAud = aud
|
||||
@@ -43,11 +48,42 @@ export async function getServiceAuthToken({
|
||||
const {token} = await client.call(com.atproto.server.getServiceAuth, {
|
||||
aud: resolvedAud as DidString,
|
||||
lxm,
|
||||
exp,
|
||||
exp: exp === undefined ? undefined : toIntegerExp(exp),
|
||||
})
|
||||
return token
|
||||
}
|
||||
|
||||
/**
|
||||
* Default lifetime for the video upload service auth token. Long enough to
|
||||
* cover a slow upload of a large file, short enough to limit the damage if the
|
||||
* token leaks.
|
||||
*/
|
||||
export const SERVICE_AUTH_TTL_SEC = 60 * 30
|
||||
|
||||
/**
|
||||
* Build a service auth `exp` claim `ttlSec` seconds from now.
|
||||
*
|
||||
* Always use this instead of hand-rolling the arithmetic: `Date.now()` is in
|
||||
* milliseconds, and dividing by 1000 without flooring yields a fractional
|
||||
* timestamp that the endpoint rejects.
|
||||
*/
|
||||
export function serviceAuthExp(ttlSec: number = SERVICE_AUTH_TTL_SEC) {
|
||||
return Math.floor(Date.now() / 1000) + Math.floor(ttlSec)
|
||||
}
|
||||
|
||||
/**
|
||||
* The lexicon types `exp` as an integer and it is serialized straight into the
|
||||
* query string, so a fractional value fails validation and the upload dies
|
||||
* before it starts. Floor here, at the single chokepoint every caller goes
|
||||
* through, so a call site that forgets to cannot reintroduce the bug.
|
||||
*/
|
||||
function toIntegerExp(exp: number) {
|
||||
if (!Number.isFinite(exp)) {
|
||||
throw new Error(`Invalid service auth exp: ${exp}`)
|
||||
}
|
||||
return Math.floor(exp)
|
||||
}
|
||||
|
||||
export async function getVideoUploadLimits(client: Client, i18n: I18n) {
|
||||
const token = await getServiceAuthToken({
|
||||
client,
|
||||
|
||||
@@ -13,7 +13,11 @@ import {
|
||||
import {Features, features} from '#/analytics/features'
|
||||
import {type app} from '#/lexicons'
|
||||
import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload'
|
||||
import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared'
|
||||
import {
|
||||
getServiceAuthToken,
|
||||
getVideoUploadLimits,
|
||||
serviceAuthExp,
|
||||
} from './upload.shared'
|
||||
import {createVideoEndpointUrl, mimeToExt} from './util'
|
||||
|
||||
export async function uploadVideo({
|
||||
@@ -72,7 +76,7 @@ export async function uploadVideo({
|
||||
client,
|
||||
dispatchUrl,
|
||||
lxm: 'com.atproto.repo.uploadBlob',
|
||||
exp: Date.now() / 1000 + 60 * 30, // 30 minutes
|
||||
exp: serviceAuthExp(),
|
||||
})
|
||||
const uploadTask = createUploadTask(
|
||||
uri,
|
||||
|
||||
@@ -12,7 +12,11 @@ import {
|
||||
import {Features, features} from '#/analytics/features'
|
||||
import {type app} from '#/lexicons'
|
||||
import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload'
|
||||
import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared'
|
||||
import {
|
||||
getServiceAuthToken,
|
||||
getVideoUploadLimits,
|
||||
serviceAuthExp,
|
||||
} from './upload.shared'
|
||||
import {createVideoEndpointUrl, mimeToExt} from './util'
|
||||
|
||||
export async function uploadVideo({
|
||||
@@ -79,7 +83,7 @@ export async function uploadVideo({
|
||||
client,
|
||||
dispatchUrl,
|
||||
lxm: 'com.atproto.repo.uploadBlob',
|
||||
exp: Date.now() / 1000 + 60 * 30, // 30 minutes
|
||||
exp: serviceAuthExp(),
|
||||
})
|
||||
|
||||
if (signal.aborted) {
|
||||
|
||||
Reference in New Issue
Block a user