Crash when a third-party firewall refused to hand over a favicon.ico file

Open Beginner friendly
#858 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
86/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
php
Domain
backend

Research direction

Start in passman/lib/Controller/IconController.php, especially the icon response handling around lines 58 and 93. Reproduce the restricted favicon.ico case from the listed steps and inspect how missing icon fields are handled. Done means the request no longer crashes when the firewall refuses the favicon, while normal icon responses still work.

Written by the indexing model from the issue text.

Description

bug
How to use GitHub
  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.
Steps to reproduce
  1. Create a new credential or go to edit an existing one
  2. Add an URL with restricted favicon.ico (for example https://www.autodesk.com/)
  3. In the label section select the icon and from the "Pick an icon" window select "Get icon from page".
  4. The Nextcloud log file will present an error
Expected behaviour

The expectation is PassMan to ignore the icon request if a third-party firewall refuse to hand over a favicon.ico file.

Solution (valid for PHP 8.0+)

Open the file ../passman/lib/Controller/IconController.php

Go to line 58 and find

if ($icon->icoExists) {
  $icon_json['type'] = $icon->icoType;
  $icon_json['content'] = base64_encode($icon->icoData);
  return new JSONResponse($icon_json);
}

Replace it with

if (isset($icon->icoExists) && $icon->icoExists) {
  $icon_json['type'] = isset($icon->icoType) ? $icon->icoType : 'png';
  $icon_json['content'] = isset($icon->icoData) ? base64_encode($icon->icoData) : '';
  return new JSONResponse($icon_json);
}

Go to line 93 and find

if ($icon->icoExists) {
  $data = $icon->icoData;
  $type = $icon->icoType;
}

Replace it with

if (isset($icon->icoExists) && $icon->icoExists) {
  if (isset($icon->icoData)) {
    $data = $icon->icoData;
  }
  if (isset($icon->icoType)) {
    $type = $icon->icoType;
  }
}
Dominant language
JavaScript
Stars
822
Forks
118
Avg merge
1d 21h
Merged PRs (30d)
8

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from nextcloud/passman

All issues in nextcloud/passman

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.