{"action":"created","issue":{"url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/1044","repository_url":"https://api.github.com/repos/Microsoft/web-build-tools","labels_url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/1044/labels{/name}","comments_url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/1044/comments","events_url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/1044/events","html_url":"https://github.com/Microsoft/web-build-tools/issues/1044","id":401144662,"node_id":"MDU6SXNzdWU0MDExNDQ2NjI=","number":1044,"title":"[api-extractor] PackageMetadataManager.isAedocSupportedFor is not safe","user":{"login":"shlomiassaf","id":5377501,"node_id":"MDQ6VXNlcjUzNzc1MDE=","avatar_url":"https://avatars1.githubusercontent.com/u/5377501?v=4","gravatar_id":"","url":"https://api.github.com/users/shlomiassaf","html_url":"https://github.com/shlomiassaf","followers_url":"https://api.github.com/users/shlomiassaf/followers","following_url":"https://api.github.com/users/shlomiassaf/following{/other_user}","gists_url":"https://api.github.com/users/shlomiassaf/gists{/gist_id}","starred_url":"https://api.github.com/users/shlomiassaf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shlomiassaf/subscriptions","organizations_url":"https://api.github.com/users/shlomiassaf/orgs","repos_url":"https://api.github.com/users/shlomiassaf/repos","events_url":"https://api.github.com/users/shlomiassaf/events{/privacy}","received_events_url":"https://api.github.com/users/shlomiassaf/received_events","type":"User","site_admin":false},"labels":[{"id":452461229,"node_id":"MDU6TGFiZWw0NTI0NjEyMjk=","url":"https://api.github.com/repos/Microsoft/web-build-tools/labels/needs%20more%20info","name":"needs more info","color":"d876e3","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2019-01-20T22:10:39Z","updated_at":"2019-02-04T20:12:37Z","closed_at":null,"author_association":"NONE","body":"The method [PackageMetadataManager.isAedocSupportedFor](https://github.com/Microsoft/web-build-tools/blob/master/apps/api-extractor/src/analyzer/PackageMetadataManager.ts#L138) is using `PackageJsonLookup` from `@microsoft/node-core-library` to lookup and load the `package.json` file. (through `PackageMetadataManager.tryFetchPackageMetadata`)\r\n\r\nThis seems to be safe, as long as the file does not exist.\r\n\r\nIf the file exists but the [`name` or `version` fields are missing in `package.json` the process will throw](https://github.com/Microsoft/web-build-tools/blob/master/libraries/node-core-library/src/PackageJsonLookup.ts#L200-L207)\r\n\r\nA try catch should be in-place, assuming it should be a safe pass.\r\n\r\nThis is my current workaround, not sure it cover all scenarios:\r\n\r\n```ts\r\nimport { PackageMetadataManager } from '@microsoft/api-extractor/lib/analyzer/PackageMetadataManager';\r\n\r\nconst { isAedocSupportedFor } = PackageMetadataManager.prototype;\r\n\r\nPackageMetadataManager.prototype.isAedocSupportedFor = function (sourceFilePath: string) {\r\n  try {\r\n    return isAedocSupportedFor.call(this, sourceFilePath);\r\n  } catch (err) {\r\n    return false;\r\n  }\r\n};\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/comments/460395238","html_url":"https://github.com/Microsoft/web-build-tools/issues/1044#issuecomment-460395238","issue_url":"https://api.github.com/repos/Microsoft/web-build-tools/issues/1044","id":460395238,"node_id":"MDEyOklzc3VlQ29tbWVudDQ2MDM5NTIzOA==","user":{"login":"octogonz","id":4673363,"node_id":"MDQ6VXNlcjQ2NzMzNjM=","avatar_url":"https://avatars0.githubusercontent.com/u/4673363?v=4","gravatar_id":"","url":"https://api.github.com/users/octogonz","html_url":"https://github.com/octogonz","followers_url":"https://api.github.com/users/octogonz/followers","following_url":"https://api.github.com/users/octogonz/following{/other_user}","gists_url":"https://api.github.com/users/octogonz/gists{/gist_id}","starred_url":"https://api.github.com/users/octogonz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/octogonz/subscriptions","organizations_url":"https://api.github.com/users/octogonz/orgs","repos_url":"https://api.github.com/users/octogonz/repos","events_url":"https://api.github.com/users/octogonz/events{/privacy}","received_events_url":"https://api.github.com/users/octogonz/received_events","type":"User","site_admin":false},"created_at":"2019-02-04T20:12:37Z","updated_at":"2019-02-04T20:12:37Z","author_association":"COLLABORATOR","body":"@shlomiassaf does **@<!---->angular/material** have a good reason to publish **package.json** files that do not conform to the spec?  If not, perhaps we should simply open an issue/PR to get Angular fixed.  It's probably a small change.  I don't see why they would object to such a proposal.\r\n\r\nMy concern is that if our library accepts invalid inputs, every consumer of the library will now need to be updated to check for that edge case and handle it somehow.  This increases the complexity and test surface for our open source projects.  If there is a good reason for omitting the `version` field, then I'm fine with taking on that additional cost. But the reason is merely to accommodation one particular library that doesn't follow the rules, then it seems like we're fixing the wrong end of the problem.\r\n\r\nWhat do you think?"}}
{"actor":{"display_login":"octogonz"}}