Skip to content

Commit

Permalink
Fixes issue #2208 (#2215)
Browse files Browse the repository at this point in the history
* Fixes issue #2208

* Code review comments

* Code review comments

* Code review comments

* Code review comments

* Code review comments

* Code cleanup

* Tests fail on local dev - so using Azure devops to test if these are passing there

* Testing changes in Azure Devops

* Canonicalized Resource string bug fixed

* Fixed bug in createStringToSignForSharedKeyLite routine

* Fixes bug in createStringToSignForSharedKeyLite

* Clean up and documentation
  • Loading branch information
vigneashs authored Oct 19, 2023
1 parent 3e65074 commit 68a3b05
Show file tree
Hide file tree
Showing 9 changed files with 333 additions and 17 deletions.
3 changes: 3 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ Blob:
- Fixed startCopyFromURL, copyFromURL API to return 400 (InvalidHeaderValue) when copy source has invalid format. (issue #1954)
- Fixed CommitBlockList API to return 400 (InvalidXmlDocument) when the request is sent with JSON body. (issue #1955)
- Added "x-ms-is-hns-enabled" header in x-ms-is-hns-enabled API responds (issue #1810)
- Fixed authentication error in production style URL for secondary location (issue #2208)

Queue:

- Fixed set Queue ACL failure when Start is missing (issue #2065)
- Fixed authentication error in production style URL for secondary location (issue #2208)

Table:

- Fixed the errorCode returned, when malformed Etag is provided for table Update/Delete calls. (issue #2013)
- Fixed an issue when comparing `'' eq guid'00000000-0000-0000-0000-000000000000'` which would erroneously report these as equal. (issue #2169)
- Fixed authentication error in production style URL for secondary location (issue #2208)

## 2023.08 Version 3.26.0

Expand Down
10 changes: 8 additions & 2 deletions src/blob/middlewares/blobStorageContext.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,12 @@ export function internnalBlobStorageContextMiddleware(
blobContext.authenticationPath = reqPath;
if (isSecondary) {
const pos = blobContext.authenticationPath!.search(SECONDARY_SUFFIX);
blobContext.authenticationPath =
if (pos !== -1)
{
blobContext.authenticationPath =
blobContext.authenticationPath!.substr(0, pos) +
blobContext.authenticationPath!.substr(pos + SECONDARY_SUFFIX.length);
}
}

if (!account) {
Expand Down Expand Up @@ -200,9 +203,12 @@ export function blobStorageContextMiddleware(
blobContext.authenticationPath = req.path;
if (isSecondary) {
const pos = blobContext.authenticationPath.search(SECONDARY_SUFFIX);
blobContext.authenticationPath =
if (pos !== -1)
{
blobContext.authenticationPath =
blobContext.authenticationPath.substr(0, pos) +
blobContext.authenticationPath.substr(pos + SECONDARY_SUFFIX.length);
}
}

if (!account) {
Expand Down
5 changes: 4 additions & 1 deletion src/queue/middlewares/queueStorageContext.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,12 @@ export function queueStorageContextMiddleware(
queueContext.authenticationPath = req.path;
if (isSecondary) {
const pos = queueContext.authenticationPath.search(SECONDARY_SUFFIX);
queueContext.authenticationPath =
if (pos !== -1)
{
queueContext.authenticationPath =
queueContext.authenticationPath.substr(0, pos) +
queueContext.authenticationPath.substr(pos + SECONDARY_SUFFIX.length);
}
}

if (account === undefined) {
Expand Down
5 changes: 4 additions & 1 deletion src/table/middleware/tableStorageContext.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,12 @@ export function tableStorageContextMiddleware(

if (isSecondary) {
const pos = tableContext.authenticationPath.search(SECONDARY_SUFFIX);
tableContext.authenticationPath =
if (pos !== -1)
{
tableContext.authenticationPath =
tableContext.authenticationPath.substr(0, pos) +
tableContext.authenticationPath.substr(pos + SECONDARY_SUFFIX.length);
}
}

// Emulator's URL pattern is like http://hostname[:port]/account/table
Expand Down
68 changes: 67 additions & 1 deletion tests/blob/specialnaming.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ describe("SpecialNaming", () => {
const server = factory.createServer();

const baseURL = `http://${server.config.host}:${server.config.port}/devstoreaccount1`;
const productionStyleHostName = "devstoreaccount1.localhost"; // Use hosts file to make this resolve
const baseSecondaryURL = `http://${server.config.host}:${server.config.port}/devstoreaccount1-secondary`;
const productionStyleHostName = "devstoreaccount1.blob.localhost"; // Use hosts file to make this resolve
const productionStyleHostNameForSecondary = "devstoreaccount1-secondary.blob.localhost";
const noAccountHostName = "host.docker.internal";
const noAccountHostNameConnectionString = `DefaultEndpointsProtocol=http;AccountName=${EMULATOR_ACCOUNT_NAME};AccountKey=${EMULATOR_ACCOUNT_KEY};BlobEndpoint=http://${noAccountHostName}:${server.config.port}/${EMULATOR_ACCOUNT_NAME};`;

Expand Down Expand Up @@ -540,4 +542,68 @@ describe("SpecialNaming", () => {
}
);
});

it(`Should work with production style URL when ${productionStyleHostNameForSecondary} is resolvable`, async () => {
await dns.promises.lookup(productionStyleHostNameForSecondary).then(
async (lookupAddress) => {
const baseURLProductionStyle = `http://${productionStyleHostNameForSecondary}:${server.config.port}`;
const serviceClientProductionStyle = new BlobServiceClient(
baseURLProductionStyle,
newPipeline(
new StorageSharedKeyCredential(
EMULATOR_ACCOUNT_NAME,
EMULATOR_ACCOUNT_KEY
),
{
retryOptions: { maxTries: 1 },
// Make sure socket is closed once the operation is done.
keepAliveOptions: { enable: false }
}
)
);
const containerClientProductionStyle = serviceClientProductionStyle.getContainerClient(
containerName
);

const response =
await containerClientProductionStyle.getProperties();

assert.deepStrictEqual(response._response.status, 200);
},
() => {
// Cannot perform this test. We need devstoreaccount1-secondary.blob.localhost to resolve to 127.0.0.1.
// On Linux, this should just work,
// On Windows, we can't spoof DNS record for specific process.
// So we have options of running our own DNS server (overkill),
// or editing hosts files (machine global operation; and requires running as admin).
// So skip the test case.
assert.ok(
`Skipping test case - it needs ${productionStyleHostNameForSecondary} to be resolvable`
);
}
);
});

it(`Should work with non-production secondary url when ${baseSecondaryURL} is resolvable`, async () => {
const secondaryServiceClient = new BlobServiceClient(
baseSecondaryURL,
newPipeline(
new StorageSharedKeyCredential(
EMULATOR_ACCOUNT_NAME,
EMULATOR_ACCOUNT_KEY
),
{
retryOptions: { maxTries: 1 },
// Make sure socket is closed once the operation is done.
keepAliveOptions: { enable: false }
}
)
);
const containerClientSecondary = secondaryServiceClient.getContainerClient(
getUniqueName("container")
);

const response = await containerClientSecondary.createIfNotExists();
assert.deepStrictEqual(response._response.status, 201);
});
});
110 changes: 110 additions & 0 deletions tests/queue/queueSpecialnaming.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as assert from "assert";
import dns = require("dns");

import {
newPipeline,
Expand All @@ -13,6 +14,7 @@ import Server from "../../src/queue/QueueServer";
import {
EMULATOR_ACCOUNT_KEY,
EMULATOR_ACCOUNT_NAME,
getUniqueName,
rmRecursive
} from "../testutils";

Expand Down Expand Up @@ -45,6 +47,10 @@ describe("Queue SpecialNaming", () => {
);

const baseURL = `http://${host}:${port}/devstoreaccount1`;
const baseSecondaryURL = `http://${host}:${port}/devstoreaccount1-secondary`;
const productionStyleHostName = "devstoreaccount1.queue.localhost"; // Use hosts file to make this resolve
const productionStyleHostNameForSecondary = "devstoreaccount1-secondary.queue.localhost";

const serviceClient = new QueueServiceClient(
baseURL,
newPipeline(
Expand Down Expand Up @@ -187,4 +193,108 @@ describe("Queue SpecialNaming", () => {
)
);
});

it(`Should work with production style URL when ${productionStyleHostName} is resolvable`, async () => {
let queueName = getUniqueName("queue");
await dns.promises.lookup(productionStyleHostName).then(
async (lookupAddress) => {
const baseURLProductionStyle = `http://${productionStyleHostName}:${port}`;
const serviceClientProductionStyle = new QueueServiceClient(
baseURLProductionStyle,
newPipeline(
new StorageSharedKeyCredential(
EMULATOR_ACCOUNT_NAME,
EMULATOR_ACCOUNT_KEY
),
{
retryOptions: { maxTries: 1 },
// Make sure socket is closed once the operation is done.
keepAliveOptions: { enable: false }
}
)
);
const queueProductionStyle = serviceClientProductionStyle.getQueueClient(
queueName
);

const response = await queueProductionStyle.create();
assert.deepStrictEqual(response._response.status, 201);
await queueProductionStyle.delete();
},
() => {
// Cannot perform this test. We need devstoreaccount1-secondary.blob.localhost to resolve to 127.0.0.1.
// On Linux, this should just work,
// On Windows, we can't spoof DNS record for specific process.
// So we have options of running our own DNS server (overkill),
// or editing hosts files (machine global operation; and requires running as admin).
// So skip the test case.
assert.ok(
`Skipping test case - it needs ${productionStyleHostNameForSecondary} to be resolvable`
);
}
);
});

it(`Should work with production style URL when ${productionStyleHostNameForSecondary} is resolvable`, async () => {
let queueName = getUniqueName("queue");
await dns.promises.lookup(productionStyleHostNameForSecondary).then(
async (lookupAddress) => {
const baseURLProductionStyle = `http://${productionStyleHostNameForSecondary}:${port}`;
const serviceClientProductionStyle = new QueueServiceClient(
baseURLProductionStyle,
newPipeline(
new StorageSharedKeyCredential(
EMULATOR_ACCOUNT_NAME,
EMULATOR_ACCOUNT_KEY
),
{
retryOptions: { maxTries: 1 },
// Make sure socket is closed once the operation is done.
keepAliveOptions: { enable: false }
}
)
);
const queueProductionStyle = serviceClientProductionStyle.getQueueClient(
queueName
);

const response = await queueProductionStyle.create();
assert.deepStrictEqual(response._response.status, 201);
await queueProductionStyle.delete();
},
() => {
// Cannot perform this test. We need devstoreaccount1-secondary.blob.localhost to resolve to 127.0.0.1.
// On Linux, this should just work,
// On Windows, we can't spoof DNS record for specific process.
// So we have options of running our own DNS server (overkill),
// or editing hosts files (machine global operation; and requires running as admin).
// So skip the test case.
assert.ok(
`Skipping test case - it needs ${productionStyleHostNameForSecondary} to be resolvable`
);
}
);
});

it(`Should work with non-production secondary url when ${baseSecondaryURL} is resolvable`, async () => {
const secondaryServiceClient = new QueueServiceClient(
baseSecondaryURL,
newPipeline(
new StorageSharedKeyCredential(
EMULATOR_ACCOUNT_NAME,
EMULATOR_ACCOUNT_KEY
),
{
retryOptions: { maxTries: 1 },
// Make sure socket is closed once the operation is done.
keepAliveOptions: { enable: false }
}
)
);
let queueName = getUniqueName("queue");
const queueClientSecondary = secondaryServiceClient.getQueueClient(queueName);
const response = await queueClientSecondary.create();
assert.deepStrictEqual(response._response.status, 201);
await queueClientSecondary.delete();
});
});
81 changes: 75 additions & 6 deletions tests/table/apis/table.validation.rest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@
// using the SDKs, can be used as a test rig for repros which provide a debug log.
// later we can automate the parsing of repro logs to automatically play these into the tester
// special care is needed to replace etags and folders when used

import * as assert from "assert";
import { configLogger } from "../../../src/common/Logger";
import TableConfiguration from "../../../src/table/TableConfiguration";
import TableServer from "../../../src/table/TableServer";
import { getUniqueName } from "../../testutils";
import { getUniqueName } from "../../testutils";
import {
deleteToAzurite,
getToAzurite,
postToAzurite
postToAzurite,
postToAzuriteProductionUrl,
getToAzuriteProductionUrl
} from "../utils/table.entity.tests.rest.submitter";
import dns = require("dns");

// Set true to enable debug log
configLogger(false);
Expand All @@ -31,10 +33,10 @@ describe("table name validation tests", () => {
enableDebugLog,
false,
undefined,
debugLogPath,
false,
true
debugLogPath
);
const productionStyleHostName = "devstoreaccount1.table.localhost"; // Use hosts file to make this resolve
const productionStyleHostNameForSecondary = "devstoreaccount1-secondary.table.localhost";

let server: TableServer;

Expand Down Expand Up @@ -398,4 +400,71 @@ describe("table name validation tests", () => {
);
}
});


it(`Should work with production style URL when ${productionStyleHostName} is resolvable`, async () => {
await dns.promises.lookup(productionStyleHostName).then(
async (lookupAddress) => {
let tableName = getUniqueName("table");
const body = JSON.stringify({
TableName: tableName
});
const createTableHeaders = {
"Content-Type": "application/json",
Accept: "application/json;odata=nometadata"
};
try {
let response = await postToAzuriteProductionUrl(productionStyleHostName,"Tables", body, createTableHeaders);
assert.strictEqual(response.status, 201);
} catch (err: any) {
assert.fail();
}
},
() => {
// Cannot perform this test. We need devstoreaccount1-secondary.blob.localhost to resolve to 127.0.0.1.
// On Linux, this should just work,
// On Windows, we can't spoof DNS record for specific process.
// So we have options of running our own DNS server (overkill),
// or editing hosts files (machine global operation; and requires running as admin).
// So skip the test case.
assert.ok(
`Skipping test case - it needs ${productionStyleHostName} to be resolvable`
);
}
);
});

it(`Should work with production style URL when ${productionStyleHostNameForSecondary} is resolvable`, async () => {
await dns.promises.lookup(productionStyleHostNameForSecondary).then(
async (lookupAddress) => {
let tableName = getUniqueName("table");
const body = JSON.stringify({
TableName: tableName
});
const createTableHeaders = {
"Content-Type": "application/json",
Accept: "application/json;odata=nometadata"
};
try {
let response = await postToAzuriteProductionUrl(productionStyleHostName,"Tables", body, createTableHeaders);
assert.strictEqual(response.status, 201);
let tablesList = await getToAzuriteProductionUrl(productionStyleHostNameForSecondary,"Tables", createTableHeaders);
assert.strictEqual(tablesList.status, 200);
} catch (err: any) {
assert.fail();
}
},
() => {
// Cannot perform this test. We need devstoreaccount1-secondary.blob.localhost to resolve to 127.0.0.1.
// On Linux, this should just work,
// On Windows, we can't spoof DNS record for specific process.
// So we have options of running our own DNS server (overkill),
// or editing hosts files (machine global operation; and requires running as admin).
// So skip the test case.
assert.ok(
`Skipping test case - it needs ${productionStyleHostNameForSecondary} to be resolvable`
);
}
);
});
});
Loading

0 comments on commit 68a3b05

Please sign in to comment.