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

allows loading of icons via http scheme #177

Merged
merged 1 commit into from
Oct 9, 2016

Conversation

boneskull
Copy link
Contributor

  • Key NSAppTransportSecurity disallows http by default, and requires https
  • Some usable icon resources (loaded via -appIcon or otherwise) are not readily accessible via http

Before:

$ terminal-notifier -t Homebrew -message 'Homebrew upgraded!' -timeout 10 -open file:///tmp/org.boneskull.homebrew-upgrade.log -group org.boneskull.hombrew-upgrade -appIcon http://brew.sh/img/homebrew-256x256.png
2016-10-06 10:43:57.173 terminal-notifier[9174:16746114] App Transport Security has blocked a cleartext HTTP (http://) resource load since it is insecure. Temporary exceptions can be configured via your app's Info.plist file.

screenshot

After:

$ terminal-notifier -t Homebrew -message 'Homebrew upgraded!' -timeout 10 -open file:///tmp/org.boneskull.homebrew-upgrade.log -group org.boneskull.hombrew-upgrade -appIcon http://brew.sh/img/homebrew-256x256.png -t Homebrew -message 'Homebrew upgraded!' -timeout 10 -open file:///tmp/org.boneskull.homebrew-upgrade.log -group org.boneskull.hombrew-upgrade -appIcon http://brew.sh/img/homebrew-256x256.png

screenshot


Caveat: I'm ignorant about what potential problems this could cause otherwise. In my limited testing, the setting NSAllowsArbitraryLoadsInWebContent was insufficient to allow the resource. I didn't try anything else.

Sorry to waste your time if this has been addressed before. I also understand if you think it's not a good idea.

- Key `NSAppTransportSecurity` disallows `http` by default, and requires `https`
- Some usable icon resources (loaded via `-appIcon` or otherwise) are not readily accessible via `http`
@julienXX julienXX merged commit f7db151 into julienXX:master Oct 9, 2016
@julienXX
Copy link
Owner

julienXX commented Oct 9, 2016

Thanks! Since terminal-notifier is already not suitable for MAS release, I guess it's okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants