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

Allow any weak etag even if it doesn't match the pattern (412 should happen) #1231

Merged
merged 4 commits into from
Feb 16, 2022
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
4 changes: 4 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ Queue:

- Fixed issue that queue list result is not in alphabetical order.

Table:

- Allow any valid weak etag even though we know it will fail with a 412

## 2021.10 Version 3.15.0

General:
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
"watch": "tsc -watch -p ./",
"blob": "node -r ts-node/register src/blob/main.ts",
"queue": "node -r ts-node/register src/queue/main.ts",
"table": "node -r ts-node/register src/table/main.ts",
"azurite": "node -r ts-node/register src/azurite.ts",
"lint": "tslint -p tsconfig.json -c tslint.json src/**/*.ts",
"test": "npm run lint && cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 mocha --compilers ts-node/register --no-timeouts --grep @loki --recursive tests/**/*.test.ts tests/**/**/*.test.ts",
Expand Down
24 changes: 0 additions & 24 deletions src/common/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,6 @@ export function newEtag(): string {
);
}

export function newTableEntityEtag(startTime: Date): string {
// Etag as returned by Table Storage should match W/"datetime'<ISO8601datetime>'"
return (
"W/\"datetime'" +
encodeURIComponent(truncatedISO8061Date(startTime, true)) +
"'\""
);
}

/**
* Generates a hash signature for an HTTP request or for a SAS.
*
Expand Down Expand Up @@ -160,18 +151,3 @@ export async function getMD5FromStream(
});
});
}

/**
* Checks if an eTag is valid
*
* @export
* @param {string} etag
* @return {*} {boolean}
*/
export function checkEtagIsInvalidFormat(etag: string): boolean {
// Etag should match ^W\/"datetime'\d{4}-\d{2}-\d{2}T\d{2}%3A\d{2}%3A\d{2}.\d{7}Z'"$
const match = etag.match(
/^W\/"datetime'\d{4}-\d{2}-\d{2}T\d{2}%3A\d{2}%3A\d{2}.\d{7}Z'"$/
);
return match === null;
}
2 changes: 1 addition & 1 deletion src/table/handlers/TableHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import BufferStream from "../../common/utils/BufferStream";
import {
checkEtagIsInvalidFormat,
newTableEntityEtag
} from "../../common/utils/utils";
} from "../utils/utils";
import TableBatchOrchestrator from "../batch/TableBatchOrchestrator";
import TableBatchUtils from "../batch/TableBatchUtils";
import TableStorageContext from "../context/TableStorageContext";
Expand Down
2 changes: 1 addition & 1 deletion src/table/persistence/LokiTableMetadataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ export default class LokiTableMetadataStore implements ITableMetadataStore {
}

// if match is URL encoded from the clients, match URL encoding
// this does not always seem to be consisten...
// this does not always seem to be consistent...
const encodedEtag = doc.eTag.replace(":", "%3A").replace(":", "%3A");
let encodedIfMatch: string | undefined;
if (ifMatch !== undefined) {
Expand Down
26 changes: 26 additions & 0 deletions src/table/utils/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { truncatedISO8061Date } from "../../common/utils/utils";
import StorageErrorFactory from "../errors/StorageErrorFactory";
import { TableResponseProperties } from "../generated/artifacts/models";
import Context from "../generated/Context";
Expand Down Expand Up @@ -269,3 +270,28 @@ export function validateTableName(context: Context, tableName: string) {
throw StorageErrorFactory.getInvalidResourceName(context);
}
}

export function newTableEntityEtag(startTime: Date): string {
// Etag as returned by Table Storage should match W/"datetime'<ISO8601datetime>'"
return (
"W/\"datetime'" +
encodeURIComponent(truncatedISO8061Date(startTime, true)) +
"'\""
);
}

/**
* Checks if an eTag is valid
*
* @export
* @param {string} etag
* @return {*} {boolean}
*/
export function checkEtagIsInvalidFormat(etag: string): boolean {
// Weak etag is required. This is parity with Azure and legacy emulator.
// Source for regex: https://stackoverflow.com/a/11572348
const match = etag.match(
/^[wW]\/"([^"]|\\")*"$/
);
return match === null;
}
4 changes: 2 additions & 2 deletions tests/table/apis/table.entity.test.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function createSecondaryConnectionStringForTest(dev: boolean): string {
* @export
* @return {*} {string}
*/
export function createUniquePartitionKey(name: string | undefined): string {
export function createUniquePartitionKey(name?: string | undefined): string {
if (name === undefined) {
return getUniqueName("datatablestests");
}
Expand Down Expand Up @@ -139,7 +139,7 @@ export function createAzureDataTablesClient(
} else {
return new TableClient(
process.env[AZURE_DATATABLES_STORAGE_STRING]! +
process.env[AZURE_DATATABLES_SAS]!,
process.env[AZURE_DATATABLES_SAS]!,
tableName
);
}
Expand Down
71 changes: 71 additions & 0 deletions tests/table/apis/table.entity.tests.issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,77 @@ describe("table Entity APIs test", () => {
await server.close();
});

// from issue #1229
[
"W/\"wrong\"",
"W/\"datetime'2015-01-01T23%3A14%3A33.4980000Z'\"",
"w/\"wrong\"",
"w/\"datetime'2015-01-01T23%3A14%3A33.4980000Z'\"",
].forEach(etag => {
it(`should allow any valid weak etag <${etag}>, @loki`, async () => {
const partitionKey = createUniquePartitionKey();
const tableClient = createAzureDataTablesClient(
testLocalAzuriteInstance,
tableName
);
await tableClient.createTable();

const entity = {
partitionKey: partitionKey,
rowKey: "rk",
}
const response = await tableClient.createEntity(entity);

try {
await tableClient.updateEntity(entity, 'Replace', { etag })
assert.fail();
} catch (error: any) {
assert.strictEqual(error.statusCode, 412);
}

const existing = await tableClient.getEntity(entity.partitionKey, entity.rowKey)
assert.strictEqual(response.etag, existing.etag)

await tableClient.deleteTable();
});
});

// from issue #1229
[
"\"wrong\"",
"\"datetime'2015-01-01T23%3A14%3A33.4980000Z'\"",
"wrong",
"datetime'2015-01-01T23%3A14%3A33.4980000Z'",
"\"",
].forEach(etag => {
it(`should reject invalid or strong etag <${etag}>, @loki`, async () => {
const partitionKey = createUniquePartitionKey();
const tableClient = createAzureDataTablesClient(
testLocalAzuriteInstance,
tableName
);
await tableClient.createTable();

const entity = {
partitionKey: partitionKey,
rowKey: "rk",
}
const response = await tableClient.createEntity(entity);

try {
await tableClient.updateEntity(entity, 'Replace', { etag })
assert.fail();
} catch (error: any) {
assert.strictEqual(error.statusCode, 400);
}

const existing = await tableClient.getEntity(entity.partitionKey, entity.rowKey)
assert.strictEqual(response.etag, existing.etag)

await tableClient.deleteTable();
});
});

// from issue #1003
it("should return 101 entities from a paged query at 50 entities per page and single partition, @loki", async () => {
const partitionKeyForQueryTest1 = createUniquePartitionKey("1_");
Expand Down