Skip to content

Commit c35fda5

Browse files
aldomskeithrob
authored andcommitted
Fix use NuGet to modify .config (#5459) (#5460)
* Fix use NuGet to modify .config (#5459) * Encode invalid name characters in NuGetXmlHelper
1 parent b88c58a commit c35fda5

File tree

9 files changed

+97
-49
lines changed

9 files changed

+97
-49
lines changed

Tasks/Common/nuget-task-common/NuGetConfigHelper2.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import * as fs from "fs";
1+
import * as fs from "fs";
22
import * as path from "path";
33
import * as url from "url";
44
import * as Q from "q";
@@ -26,10 +26,10 @@ export class NuGetConfigHelper2 {
2626
private authInfo: auth.NuGetExtendedAuthInfo,
2727
private environmentSettings: ngToolRunner.NuGetEnvironmentSettings,
2828
private tempConfigPath: string /*optional*/,
29-
private useNuGetToModifyConfigFile?: boolean /* optional */)
29+
useNuGetToModifyConfigFile?: boolean /* optional */)
3030
{
3131
this.tempNugetConfigPath = tempConfigPath || this.getTempNuGetConfigPath();
32-
this.useNuGetToModifyConfigFile = useNuGetToModifyConfigFile || true;
32+
useNuGetToModifyConfigFile = useNuGetToModifyConfigFile === undefined ? true : useNuGetToModifyConfigFile;
3333
this.nugetXmlHelper = useNuGetToModifyConfigFile ?
3434
new NuGetExeXmlHelper(this.nugetPath, this.tempNugetConfigPath, this.authInfo, this.environmentSettings) :
3535
new NuGetXmlHelper(this.tempNugetConfigPath);

Tasks/Common/nuget-task-common/NuGetXmlHelper.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
1010
}
1111

1212
public SetApiKeyInNuGetConfig(source: string, apiKey: string): void {
13-
throw new Error(tl.loc('Error_ApiKeyNotSupported'));
13+
throw new Error(tl.loc("Error_ApiKeyNotSupported"));
1414
}
1515

1616
public AddSourceToNuGetConfig(name: string, source: string, username?: string, password?: string): void {
@@ -25,7 +25,7 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
2525

2626
if (username || password) {
2727
if (!username || !password) {
28-
tl.debug('Adding NuGet source with username and password, but one of them is missing.');
28+
tl.debug("Adding NuGet source with username and password, but one of them is missing.");
2929
}
3030

3131
xml = this._addCredentialsToSource(xml, name, username, password);
@@ -41,7 +41,7 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
4141
if (xml) {
4242
NuGetXmlHelper._validateXmlIsConfiguration(xml);
4343
let xmlSources = xml.getChildrenByFilter((child: any): boolean => {
44-
return typeof(child) === 'object' &&
44+
return typeof(child) === "object" &&
4545
child.getName().toLowerCase() === "add" &&
4646
child.up().getName().toLowerCase() === "packagesources" &&
4747
child.attrs.key === name;
@@ -78,7 +78,7 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
7878
private _removeSourceCredentials(xml: any, name: string): any {
7979
if (xml) {
8080
let xmlSourceCredentials = xml.getChildrenByFilter((child: any): boolean => {
81-
return typeof(child) === 'object' &&
81+
return typeof(child) === "object" &&
8282
child.getName() === NuGetXmlHelper._nuGetEncodeElementName(name) &&
8383
child.up().getName().toLowerCase() === "packagesourcecredentials";
8484
}, true);
@@ -98,8 +98,8 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
9898
*/
9999
private static _validateXmlIsConfiguration(xml: any): void {
100100
if (xml) {
101-
if (xml.getName().toLowerCase() !== 'configuration') {
102-
throw Error(tl.loc('Error_ExpectedConfigurationElement'));
101+
if (xml.getName().toLowerCase() !== "configuration") {
102+
throw Error(tl.loc("Error_ExpectedConfigurationElement"));
103103
}
104104
}
105105
}
@@ -139,11 +139,30 @@ export class NuGetXmlHelper implements INuGetXmlHelper {
139139
return name;
140140
}
141141

142+
// replace the following
143+
const invalidCharacters = [" ", ":"];
144+
for (let i = 1; i < name.length; i++) {
145+
if (invalidCharacters.indexOf(name[i]) >= 0) {
146+
name = name.substr(0, i) + NuGetXmlHelper._nuGetEncodeCharater(name[i]) + name.substr(i + 1);
147+
}
148+
}
149+
150+
// if the first character is a number, encode it
142151
if (isNaN(parseInt(name.charAt(0)))) {
143152
return name;
144153
}
145154

146-
let firstCharHex = name.charCodeAt(0).toString(16);
147-
return `_x00${firstCharHex}_${name.substr(1)}`;
155+
let firstCharHex = NuGetXmlHelper._nuGetEncodeCharater(name[0]);
156+
return firstCharHex + name.substr(1);
157+
}
158+
159+
private static _nuGetEncodeCharater(char: string): string {
160+
let hexValue = char.charCodeAt(0).toString(16);
161+
// pad
162+
while (hexValue.length < 4) {
163+
hexValue = "0" + hexValue;
164+
}
165+
166+
return `_x${hexValue}_`;
148167
}
149168
}

Tasks/Common/nuget-task-common/Tests/L0.ts

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import * as path from 'path';
2-
import * as assert from 'assert';
3-
import * as ma from 'vsts-task-lib/mock-answer';
4-
import * as tmrm from 'vsts-task-lib/mock-run';
5-
import * as ttm from 'vsts-task-lib/mock-test';
6-
import * as mockery from 'mockery';
7-
import { INuGetXmlHelper } from '../INuGetXmlHelper';
1+
import * as path from "path";
2+
import * as assert from "assert";
3+
import * as ma from "vsts-task-lib/mock-answer";
4+
import * as tmrm from "vsts-task-lib/mock-run";
5+
import * as ttm from "vsts-task-lib/mock-test";
6+
import * as mockery from "mockery";
7+
import { INuGetXmlHelper } from "../INuGetXmlHelper";
88

99
class MockedTask {
1010
private _proxyUrl: string;
@@ -32,93 +32,122 @@ class MockedTask {
3232
}
3333

3434
var mockedTask: MockedTask = new MockedTask();
35-
var mockedProxy: string = 'http://proxy/';
36-
var mockedUsername: string = 'mockedUsername';
37-
var mockedPassword: string = 'mockedPassword';
38-
mockery.registerMock('vsts-task-lib/task', mockedTask);
35+
var mockedProxy: string = "http://proxy/";
36+
var mockedUsername: string = "mockedUsername";
37+
var mockedPassword: string = "mockedPassword";
38+
mockery.registerMock("vsts-task-lib/task", mockedTask);
3939

40-
describe('nuget-task-common Task Suite', function () {
41-
before(() => {
40+
describe("nuget-task-common Task Suite", function() {
41+
beforeEach(() => {
4242
mockery.enable({
4343
useCleanCache: true,
44+
warnOnReplace: false,
4445
warnOnUnregistered: false
4546
});
4647
});
4748

48-
after(() => {
49+
afterEach(() => {
4950
mockery.disable();
5051
});
5152

52-
it('No HTTP_PROXY', (done:MochaDone) => {
53+
it("No HTTP_PROXY", (done: MochaDone) => {
5354
mockedTask.setMockedValues();
54-
let ngToolRunner = require('../NuGetToolRunner');
55+
let ngToolRunner = require("../NuGetToolRunner");
5556

5657
let httpProxy: string = ngToolRunner.getNuGetProxyFromEnvironment();
5758
assert.strictEqual(httpProxy, undefined);
5859

5960
done();
6061
});
6162

62-
it('Finds HTTP_PROXY', (done: MochaDone) => {
63+
it("Finds HTTP_PROXY", (done: MochaDone) => {
6364
mockedTask.setMockedValues(mockedProxy);
64-
let ngToolRunner = require('../NuGetToolRunner');
65+
let ngToolRunner = require("../NuGetToolRunner");
6566

6667
let httpProxy: string = ngToolRunner.getNuGetProxyFromEnvironment();
6768
assert.strictEqual(httpProxy, mockedProxy);
6869

6970
done();
7071
});
7172

72-
it('Finds HTTP_PROXYUSERNAME', (done: MochaDone) => {
73+
it("Finds HTTP_PROXYUSERNAME", (done: MochaDone) => {
7374
mockedTask.setMockedValues(mockedProxy, mockedUsername);
74-
let ngToolRunner = require('../NuGetToolRunner');
75+
let ngToolRunner = require("../NuGetToolRunner");
7576

7677
let httpProxy: string = ngToolRunner.getNuGetProxyFromEnvironment();
7778
assert.strictEqual(httpProxy, `http://${mockedUsername}@proxy/`);
7879

7980
done();
8081
});
8182

82-
it('Finds HTTP_PROXYPASSWORD', (done: MochaDone) => {
83+
it("Finds HTTP_PROXYPASSWORD", (done: MochaDone) => {
8384
mockedTask.setMockedValues(mockedProxy, mockedUsername, mockedPassword);
84-
let ngToolRunner = require('../NuGetToolRunner');
85+
let ngToolRunner = require("../NuGetToolRunner");
8586

8687
let httpProxy: string = ngToolRunner.getNuGetProxyFromEnvironment();
8788
assert.strictEqual(httpProxy, `http://${mockedUsername}:${mockedPassword}@proxy/`);
8889

8990
done();
9091
});
9192

92-
it('NuGetXmlHelper adds source to NuGetConfig', (done: MochaDone) => {
93+
it("NuGetXmlHelper adds source to NuGetConfig", (done: MochaDone) => {
9394
let configFile: string;
94-
mockery.registerMock('fs', {
95+
mockery.registerMock("fs", {
9596
readFileSync: () => configFile,
9697
writeFileSync: (path, content) => { configFile = content; }
9798
});
9899

99-
let nugetXmlHelper = require('../NuGetXmlHelper');
100+
let nugetXmlHelper = require("../NuGetXmlHelper");
100101
let helper: INuGetXmlHelper = new nugetXmlHelper.NuGetXmlHelper();
101102

102-
configFile = '<configuration/>';
103-
helper.AddSourceToNuGetConfig('SourceName', 'http://source/');
103+
configFile = "<configuration/>";
104+
helper.AddSourceToNuGetConfig("SourceName", "http://source/");
104105
assert.strictEqual(
105106
configFile,
106107
'<configuration><packageSources><add key="SourceName" value="http://source/"/></packageSources></configuration>',
107108
'Helper should have added the "SourceName" source');
108109

109-
helper.AddSourceToNuGetConfig('SourceCredentials', 'http://credentials', 'foo', 'bar');
110+
helper.AddSourceToNuGetConfig("SourceCredentials", "http://credentials", "foo", "bar");
110111
assert.strictEqual(
111112
configFile,
112113
'<configuration><packageSources><add key="SourceName" value="http://source/"/><add key="SourceCredentials" value="http://credentials"/></packageSources><packageSourceCredentials><SourceCredentials><add key="Username" value="foo"/><add key="ClearTextPassword" value="bar"/></SourceCredentials></packageSourceCredentials></configuration>',
113114
'Helper should have added the "SourceCredentials" source with credentials');
114115

115-
helper.RemoveSourceFromNuGetConfig('SourceCredentials');
116+
helper.RemoveSourceFromNuGetConfig("SourceCredentials");
116117
assert.strictEqual(
117118
configFile,
118119
'<configuration><packageSources><add key="SourceName" value="http://source/"/></packageSources><packageSourceCredentials/></configuration>',
119120
'Helper should have removed the "SourceCredentials" source and its credentials');
120121

121-
assert.throws(() => helper.SetApiKeyInNuGetConfig('http://ApiKeySource/', 'ApiKey'), 'SetApiKey should throw as it is not currently supported');
122+
assert.throws(() => helper.SetApiKeyInNuGetConfig("http://ApiKeySource/", "ApiKey"), "SetApiKey should throw as it is not currently supported");
123+
124+
done();
125+
});
126+
127+
it("NuGetXmlHelper correctly encodes element names", (done: MochaDone) => {
128+
let configFile: string;
129+
mockery.registerMock("fs", {
130+
readFileSync: () => configFile,
131+
writeFileSync: (path, content) => { configFile = content; }
132+
});
133+
134+
let nugetXmlHelper = require("../NuGetXmlHelper");
135+
let helper: INuGetXmlHelper = new nugetXmlHelper.NuGetXmlHelper();
136+
137+
configFile = "<configuration/>";
138+
helper.AddSourceToNuGetConfig("1Feed", "http://credentials", "foo", "bar");
139+
assert.strictEqual(
140+
configFile,
141+
'<configuration><packageSources><add key="1Feed" value="http://credentials"/></packageSources><packageSourceCredentials><_x0031_Feed><add key="Username" value="foo"/><add key="ClearTextPassword" value="bar"/></_x0031_Feed></packageSourceCredentials></configuration>',
142+
'Helper should have added the "1Feed" source with credentials');
143+
helper.RemoveSourceFromNuGetConfig("1Feed");
144+
145+
helper.AddSourceToNuGetConfig("Feed with spaces and :", "http://credentials", "foo", "bar");
146+
assert.strictEqual(
147+
configFile,
148+
'<configuration><packageSources><add key="Feed with spaces and :" value="http://credentials"/></packageSources><packageSourceCredentials><Feed_x0020_with_x0020_spaces_x0020_and_x0020__x003a_><add key="Username" value="foo"/><add key="ClearTextPassword" value="bar"/></Feed_x0020_with_x0020_spaces_x0020_and_x0020__x003a_></packageSourceCredentials></configuration>',
149+
'Helper should have added the "Feed with spaces" source with credentials');
150+
helper.RemoveSourceFromNuGetConfig("Feed with spaces");
122151

123152
done();
124153
});

Tasks/DotNetCoreCLI/Strings/resources.resjson/en-US/resources.resjson

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,4 @@
117117
"loc.messages.NGCommon_UnabletoDetectNuGetVersion": "Unknown NuGet version selected.",
118118
"loc.messages.NGCommon_UnableToFindTool": "Unable to find tool %s",
119119
"loc.messages.Warning_UpdatingNuGetVersion": "Updating version of NuGet.exe to %s from %s. Behavior changes or breaking changes might occur as NuGet updates to a new version. If this is not desired, uncheck the 'Check for Latest Version' option in the task."
120-
}
120+
}

Tasks/DotNetCoreCLI/task.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"version": {
1919
"Major": 2,
2020
"Minor": 1,
21-
"Patch": 3
21+
"Patch": 4
2222
},
2323
"minimumAgentVersion": "2.0.0",
2424
"instanceNameFormat": "dotnet $(command)",
@@ -457,4 +457,4 @@
457457
"NGCommon_UnableToFindTool": "Unable to find tool %s",
458458
"Warning_UpdatingNuGetVersion": "Updating version of NuGet.exe to %s from %s. Behavior changes or breaking changes might occur as NuGet updates to a new version. If this is not desired, uncheck the 'Check for Latest Version' option in the task."
459459
}
460-
}
460+
}

Tasks/DotNetCoreCLI/task.loc.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"version": {
1919
"Major": 2,
2020
"Minor": 1,
21-
"Patch": 3
21+
"Patch": 4
2222
},
2323
"minimumAgentVersion": "2.0.0",
2424
"instanceNameFormat": "ms-resource:loc.instanceNameFormat",
@@ -459,4 +459,4 @@
459459
"NGCommon_UnableToFindTool": "ms-resource:loc.messages.NGCommon_UnableToFindTool",
460460
"Warning_UpdatingNuGetVersion": "ms-resource:loc.messages.Warning_UpdatingNuGetVersion"
461461
}
462-
}
462+
}

Tasks/NuGetCommand/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "nugetcommand",
3-
"version": "2.0.8",
3+
"version": "2.0.9",
44
"description": "Restore, pack, or push NuGet packages, or run a NuGet command.",
55
"main": "nugetcommandmain.js",
66
"scripts": {

Tasks/NuGetCommand/task.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"version": {
1010
"Major": 2,
1111
"Minor": 0,
12-
"Patch": 8
12+
"Patch": 9
1313
},
1414
"runsOn": [
1515
"Agent",

Tasks/NuGetCommand/task.loc.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"version": {
1010
"Major": 2,
1111
"Minor": 0,
12-
"Patch": 7
12+
"Patch": 9
1313
},
1414
"runsOn": [
1515
"Agent",

0 commit comments

Comments
 (0)