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

Unhandled error in apple-touch-icon rule #515

Closed
aaronpowell opened this issue Sep 14, 2017 · 5 comments
Closed

Unhandled error in apple-touch-icon rule #515

aaronpowell opened this issue Sep 14, 2017 · 5 comments
Assignees

Comments

@aaronpowell
Copy link

aaronpowell commented Sep 14, 2017

I'm finding a crash in the apple-touch-icon rule when trying to run against my website. This was done off a base install of sonar as a local node module (rather than a global).

Environment

  • Windows 10 16281
  • sonar: 0.6.3
  • node: 8.4.0
  • npm: 5.3.0

sonar configuration

{
    "browserslist": [],
    "connector": {
        "name": "chrome",
        "options": {
            "waitFor": 1000
        }
    },
    "formatter": "stylish",
    "ignoredUrls": {},
    "rules": {
        "apple-touch-icons": "error",
        "axe": "error",
        "content-type": "error",
        "disown-opener": "error",
        "highest-available-document-mode": "error",
        "html-checker": "error",
        "manifest-app-name": "error",
        "manifest-exists": "error",
        "manifest-file-extension": "error",
        "manifest-is-valid": "error",
        "meta-charset-utf-8": "error",
        "meta-viewport": "error",
        "no-disallowed-headers": "error",
        "no-friendly-error-pages": "error",
        "no-html-only-headers": "error",
        "no-protocol-relative-urls": "error",
        "no-vulnerable-javascript-libraries": "error",
        "ssllabs": "off",
        "strict-transport-security": "error",
        "validate-set-cookie-header": "error",
        "x-content-type-options": "error"
    },
    "rulesTimeout": 120000
}

URL for which sonar failed

Output

| Traversing finishedTypeError: unsupported file type: undefined (file: undefined)
    at lookup (C:\_Projects\github\aaronpowell.github.io\node_modules\@sonarwhal\sonar\node_modules\image-size\lib\index.js:34:9)
    at module.exports (C:\_Projects\github\aaronpowell.github.io\node_modules\@sonarwhal\sonar\node_modules\image-size\lib\index.js:75:12)
    at checkImage (C:\_Projects\github\aaronpowell.github.io\node_modules\@sonarwhal\sonar\dist\src\lib\rules\apple-touch-icons\apple-touch-icons.js:89:27)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
@alrra
Copy link
Contributor

alrra commented Sep 15, 2017

Thank you for the issue, @aaronpowell!


at module.exports (C:_Projects\github\aaronpowell.github.io\node_modules@sonarwhal\sonar\node_modules\image-size\lib\index.js:75:12)

The problem is that the apple-touch-icon returns an HTML page (a 404 page) with status code 200, and image-size just chokes.

Will update the rule do some checks before using image-size.

@aaronpowell
Copy link
Author

@alrra yeah I figured it was probably a 404, so naturally telling you which URL it was would help 😉.

Interestingly enough @molant ran on his machine against the URL and didn't get the issue, but maybe he had a dev version, or the rule turned off.

@molant
Copy link
Member

molant commented Sep 15, 2017

@aaronpowell my .sonarrc didn't have that rule activated because it was generated with a previous version of sonar 😟

@alrra
Copy link
Contributor

alrra commented Sep 15, 2017

yeah I figured it was probably a 404

@aaronpowell The problem is that the site is responding with a HTML page AND the status code of 200 for an image request.

Should be fixed by #519. Thanks again for the report, keep them coming!

@alrra alrra closed this as completed in 5f43506 Sep 15, 2017
@aaronpowell
Copy link
Author

Ah good catch @alrra, I must have an overly aggressive bit of 404 handling, I should fix that on my site too!

alrra added a commit that referenced this issue Sep 15, 2017
Make `apple-touch-icons` rule gracefully handle cases when sites
respond to the image request with something other then an image,
or with a invalid or corrupt image.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix #515
Close #519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants