Skip to content

Commit

Permalink
correct filename output in reports when using zipped bundles
Browse files Browse the repository at this point in the history
  • Loading branch information
DinoChiesa committed Dec 6, 2024
1 parent 6047b6d commit cc6c320
Show file tree
Hide file tree
Showing 28 changed files with 758 additions and 707 deletions.
23 changes: 13 additions & 10 deletions lib/package/Bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ const findFolders = require("./findFolders.js"),
function _buildEndpoints(folder, tag, bundle, processFunction) {
try {
if (fs.existsSync(folder)) {
fs.readdirSync(folder).forEach(function (proxyFile) {
fs.readdirSync(folder).forEach(function (endptFile) {
//can't be a directory
//must end in .xml
const fname = path.join(folder, proxyFile);
if (proxyFile.endsWith(".xml") && fs.lstatSync(fname).isFile()) {
const fqFname = path.join(folder, endptFile);
if (endptFile.endsWith(".xml") && fs.lstatSync(fqFname).isFile()) {
const doc = xpath.select(
tag,
new Dom().parseFromString(fs.readFileSync(fname).toString())
new Dom().parseFromString(fs.readFileSync(fqFname).toString())
);
doc.forEach((element) => processFunction(element, fname, bundle));
doc.forEach((element) => processFunction(element, fqFname, bundle));
}
});
}
Expand All @@ -54,20 +54,20 @@ function _buildEndpoints(folder, tag, bundle, processFunction) {
function buildProxyEndpoints(bundle) {
var folder = bundle.proxyRoot + "/proxies/",
tag = "ProxyEndpoint",
processFunction = function (element, fname, bundle) {
processFunction = function (element, fqFname, bundle) {
bundle.proxyEndpoints = bundle.proxyEndpoints || [];
bundle.proxyEndpoints.push(new Endpoint(element, bundle, fname));
bundle.proxyEndpoints.push(new Endpoint(element, bundle, fqFname));
};
_buildEndpoints(folder, tag, bundle, processFunction);
}

function buildsharedflows(bundle) {
var folder = bundle.proxyRoot + "/sharedflows/",
tag = "SharedFlow",
processFunction = function (element, fname, bundle) {
processFunction = function (element, fqFname, bundle) {
bundle.proxyEndpoints = bundle.proxyEndpoints || [];
bundle.proxyEndpoints.push(
new Endpoint(element, bundle, fname, bundleType.BundleType.SHAREDFLOW)
new Endpoint(element, bundle, fqFname, bundleType.BundleType.SHAREDFLOW)
);
};
_buildEndpoints(folder, tag, bundle, processFunction);
Expand Down Expand Up @@ -206,7 +206,7 @@ function processFileSystem(config, bundle, cb) {
//bundle.policies = [];
//process.chdir(config.source.path);
bundle.report = {
filePath: lintUtil.effectivePath(bundle.root, bundleTypeName),
filePath: lintUtil.effectivePath(bundle, bundle.root),
errorCount: 0,
warningCount: 0,
fixableErrorCount: 0,
Expand Down Expand Up @@ -256,6 +256,9 @@ function Bundle(config, cb) {
this.excluded = config.excluded;
this.profile = config.profile;
this.ignoreDirectives = config.ignoreDirectives;
this.sourcePath = config.source.sourcePath;
this.resolvedPath = config.source.path;
this.sourceType = config.source.type;

if (config.source.type === "ManagementServer") {
//shared flow not implemented for management server
Expand Down
8 changes: 5 additions & 3 deletions lib/package/Endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@ const Flow = require("./Flow.js"),
debug = require("debug")("apigeelint:Endpoint"),
util = require("util"),
fs = require("fs"),
path = require("path"),
getcb = lintUtil.curry(lintUtil.diagcb, debug),
HTTPProxyConnection = require("./HTTPProxyConnection.js"),
HTTPTargetConnection = require("./HTTPTargetConnection.js"),
bundleTypes = require("./BundleTypes.js");

function Endpoint(element, parent, fname, bundletype) {
function Endpoint(element, bundle, fname, bundletype) {
debug(`> new Endpoint() ${fname}`);
this.bundleType = bundletype ? bundletype : "apiproxy"; //default to apiproxy if bundletype not passed
this.fileName = fname;
this.parent = parent; // the bundle
this.parent = bundle;
this.element = element;
this.preFlow = null;
this.postFlow = null;
Expand All @@ -40,7 +41,8 @@ function Endpoint(element, parent, fname, bundletype) {
this.httpProxyConnection = null;
this.httpTargetConnection = null;
this.report = {
filePath: lintUtil.effectivePath(fname, this.bundleType),
//filePath: path.join(bundle.sourcePath, path.basename(fname)),
filePath: lintUtil.effectivePath(bundle, fname),
errorCount: 0,
warningCount: 0,
fixableErrorCount: 0,
Expand Down
20 changes: 11 additions & 9 deletions lib/package/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,27 @@
*/

const fs = require("fs"),
path = require("path"),
xpath = require("xpath"),
lintUtil = require("./lintUtil.js"),
Dom = require("@xmldom/xmldom").DOMParser,
debug = require("debug")("apigeelint:Policy");

function Policy(path, fn, parent, doc) {
this.fileName = fn;
this.filePath = path + "/" + fn;
function Policy(realpath, fname, bundle, doc) {
this.fileName = fname;
const resolvedFilePath = path.join(realpath, fname);
this.filePath = resolvedFilePath;
this.bundleType =
parent && parent.bundletype ? parent.bundletype : "apiproxy";
this.parent = parent;
bundle && bundle.bundletype ? bundle.bundletype : "apiproxy";
this.parent = bundle;
if (doc) this.element = doc;
this.report = {
filePath: lintUtil.effectivePath(this.filePath, this.bundleType),
filePath: lintUtil.effectivePath(bundle, resolvedFilePath),
errorCount: 0,
warningCount: 0,
fixableErrorCount: 0,
fixableWarningCount: 0,
messages: []
messages: [],
};
}

Expand Down Expand Up @@ -93,7 +95,7 @@ Policy.prototype.getDisplayName = function () {
if (nodes[0].childNodes && nodes[0].childNodes[0]) {
this.displayName = nodes[0].childNodes[0].nodeValue;
debug(
`policy(${this.name}) DisplayName(${nodes[0].childNodes[0].nodeValue})`
`policy(${this.name}) DisplayName(${nodes[0].childNodes[0].nodeValue})`,
);
} else {
this.displayName = "";
Expand All @@ -114,7 +116,7 @@ Policy.prototype.select = function (xs) {
Policy.prototype.getElement = function () {
if (!this.element) {
this.element = new Dom().parseFromString(
fs.readFileSync(this.filePath).toString()
fs.readFileSync(this.filePath).toString(),
).documentElement;
}
return this.element;
Expand Down
12 changes: 7 additions & 5 deletions lib/package/Resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@
*/

const fs = require("fs"),
path = require("path"),
lintUtil = require("./lintUtil.js");

function Resource(parent, path, fname) {
this.parent = parent;
this.path = path;
function Resource(bundle, realpath, fname) {
this.parent = bundle;
this.path = realpath;
this.fname = fname;
this.bundleType = parent.bundletype ? parent.bundletype : "apiproxy";
this.bundleType = bundle.bundletype ? bundle.bundletype : "apiproxy";
this.messages = { warnings: [], errors: [] };
this.report = {
filePath: lintUtil.effectivePath(this.path, this.bundleType),
//filePath: path.join(bundle.sourcePath, path.basename(fname)),
filePath: lintUtil.effectivePath(bundle, realpath),
errorCount: 0,
warningCount: 0,
fixableErrorCount: 0,
Expand Down
34 changes: 18 additions & 16 deletions lib/package/lintUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ function getNodeModulesPathFor() {
const r = (acc, c) => {
if (!acc) {
let candidate = path.join(__dirname, c);
return checkCandidate(candidate)
|| checkCandidate(path.join(candidate, "node_modules"));
return (
checkCandidate(candidate) ||
checkCandidate(path.join(candidate, "node_modules"))
);
}
return acc;
};
Expand All @@ -60,7 +62,7 @@ function rBuildTagBreadCrumb(doc, bc) {
if (doc && doc.parentNode) {
bc = rBuildTagBreadCrumb(
doc.parentNode,
doc.parentNode.nodeName + ":" + bc
doc.parentNode.nodeName + ":" + bc,
);
}
return bc;
Expand Down Expand Up @@ -187,10 +189,12 @@ const getRuleId = () => {
}
};

function effectivePath(root, bundleTypeName) {
return root;
// // will this work on Windows?
// return root.substring(root.lastIndexOf("/" + bundleTypeName) + 1);
function effectivePath(bundle, fpath) {
if (bundle && bundle.sourceType == "zip") {
return fpath.replace(path.dirname(bundle.resolvedPath), bundle.sourcePath);
}

return fpath;
}

//Courtesy - https://stackoverflow.com/a/52171480
Expand Down Expand Up @@ -228,15 +232,13 @@ const xmlNodeTypeAsString = (function () {
keys = Object.keys(Node.prototype).filter((key) => key.endsWith("_NODE"));
d(`keys(${keys})`);

return (typ) =>
keys.find((key) => Node[key] == typ) || "unknown";
return (typ) => keys.find((key) => Node[key] == typ) || "unknown";
})();

const directiveRe = new RegExp("^\\s*apigeelint\\s+disable\\s?=(.+)$");
const isApigeelintDirective = (textContent) => {
const match = directiveRe.exec(textContent);
return match &&
match[1].split(",").map((s) => s.trim());
return match && match[1].split(",").map((s) => s.trim());
};

function findDirectives(elt, indent) {
Expand All @@ -246,8 +248,8 @@ function findDirectives(elt, indent) {
if (!indent) {
d(
`root: (nodeType: ${elt.nodeType}, ${xmlNodeTypeAsString(
elt.nodeType
)}) ${elt.nodeName}`
elt.nodeType,
)}) ${elt.nodeName}`,
);
}
let acc = [];
Expand All @@ -258,8 +260,8 @@ function findDirectives(elt, indent) {
const node = elt.childNodes.item(ix);
d(
`${indent}Node ${ix}: (nodeType: ${node.nodeType}, ${xmlNodeTypeAsString(
node.nodeType
)}) ${node.nodeName}`
node.nodeType,
)}) ${node.nodeName}`,
);
if (node.nodeType == Node.COMMENT_NODE) {
d(`${indent} text: ${node.textContent}`);
Expand Down Expand Up @@ -292,5 +294,5 @@ module.exports = {
findDirectives,
/* exported for testing */
getNodeModulesPathFor,
xmlNodeTypeAsString
xmlNodeTypeAsString,
};
2 changes: 1 addition & 1 deletion test/specs/PO007-PolicyNameConventions.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const testOne =
(testcase, ix, cb) => {
let policyXml = testcase,
doc = new Dom().parseFromString(policyXml),
p = new Policy(doc.documentElement, this);
p = new Policy("", `testcase-${ix}`, this, doc);
p.getElement = () => doc.documentElement;

let policyName = p.getName(),
Expand Down
100 changes: 63 additions & 37 deletions test/specs/PO007-corsPolicyName.js
Original file line number Diff line number Diff line change
@@ -1,55 +1,81 @@
// PO007-corsPolicyName.js
// ------------------------------------------------------------------
/*
Copyright 2022-2024 Google LLC
/* jshint esversion:9, node:true, strict:implied */
/* global describe, it */
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
https://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

const testID = "PO007",
assert = require("assert"),
fs = require("fs"),
path = require("path"),
bl = require("../../lib/package/bundleLinter.js"),
plugin = require(bl.resolvePlugin(testID)),
Policy = require("../../lib/package/Policy.js"),
Dom = require("@xmldom/xmldom").DOMParser;

const test =
(filename, cb) => {
it(`should correctly process ${filename}`, () => {
let fqfname = path.resolve(__dirname, '../fixtures/resources/PO007-cors-policy', filename),
policyXml = fs.readFileSync(fqfname, 'utf-8'),
doc = new Dom().parseFromString(policyXml),
p = new Policy(doc.documentElement, this);

p.getElement = () => doc.documentElement;

plugin.onPolicy(p, (e, foundIssues) => {
assert.equal(e, undefined, "should be undefined");
cb(p, foundIssues);
});
assert = require("assert"),
fs = require("fs"),
path = require("path"),
bl = require("../../lib/package/bundleLinter.js"),
plugin = require(bl.resolvePlugin(testID)),
Policy = require("../../lib/package/Policy.js"),
Dom = require("@xmldom/xmldom").DOMParser;

const test = (filename, cb) => {
it(`should correctly process ${filename}`, () => {
let baseDir = path.resolve(
__dirname,
"../fixtures/resources/PO007-cors-policy",
),
fqfname = path.resolve(baseDir, filename),
policyXml = fs.readFileSync(fqfname, "utf-8"),
doc = new Dom().parseFromString(policyXml),
p = new Policy(baseDir, fqfname, this, doc);

p.getElement = () => doc.documentElement;

plugin.onPolicy(p, (e, foundIssues) => {
assert.equal(e, undefined, "should be undefined");
cb(p, foundIssues);
});
};
});
};

describe(`PO007 - CORS policy name`, () => {

test('CORS-1.xml', (p, foundIssues) => {
test("CORS-1.xml", (p, foundIssues) => {
assert.equal(foundIssues, false);
assert.ok(p.getReport().messages, "messages undefined");
assert.equal(p.getReport().messages.length, 0, JSON.stringify(p.getReport().messages));
assert.equal(
p.getReport().messages.length,
0,
JSON.stringify(p.getReport().messages),
);
});

test('AM-CORS-1.xml', (p, foundIssues) => {
test("AM-CORS-1.xml", (p, foundIssues) => {
assert.equal(foundIssues, true);
assert.ok(p.getReport().messages, "messages undefined");
assert.equal(p.getReport().messages.length, 1, "unexpected number of messages");
assert.ok(p.getReport().messages[0].message, 'did not find message member');
assert.equal(p.getReport().messages[0].message, 'Non-standard name for policy (AM-CORS-1). Valid prefixes for the CORS policy: ["cors"]. Valid patterns: ["^cors$"].');
assert.equal(
p.getReport().messages.length,
1,
"unexpected number of messages",
);
assert.ok(p.getReport().messages[0].message, "did not find message member");
assert.equal(
p.getReport().messages[0].message,
'Non-standard name for policy (AM-CORS-1). Valid prefixes for the CORS policy: ["cors"]. Valid patterns: ["^cors$"].',
);
});

test('CORS.xml', (p, foundIssues) => {
test("CORS.xml", (p, foundIssues) => {
assert.equal(foundIssues, false);
assert.ok(p.getReport().messages, "messages undefined");
assert.equal(p.getReport().messages.length, 0, JSON.stringify(p.getReport().messages));
assert.equal(
p.getReport().messages.length,
0,
JSON.stringify(p.getReport().messages),
);
});

});
Loading

0 comments on commit cc6c320

Please sign in to comment.