Problem/Motivation

Invalid files cause exceptions instead of having NULL hash

Proposed resolution

Check if file exists

CommentFileSizeAuthor
#6 3276224-6-TEST-ONLY.patch626 bytesmfb
#5 3276224-TEST-ONLY.patch635 bytesmfb
#2 filehash_3276224-failed-to-open-stream.patch544 bytesrudi teschner

Comments

Rudi Teschner created an issue. See original summary.

rudi teschner’s picture

rudi teschner commented
StatusFileSize
new filehash_3276224-failed-to-open-stream.patch544 bytes
mfb’s picture

mfb
they or he
commented
Issue tags: +Needs tests

What exception is thrown? We should add a test for this case.

mfb’s picture

mfb
they or he
commented
Status: Active » Postponed (maintainer needs more info)

In my testing with nonexistent file, the hash is NULL, and a PHP warning is logged. This seems like correct behavior to me: If the expected file cannot be found, a warning seems reasonable. Previous versions of File Hash module silently ignored this condition rather than logging a warning, but the warning seems preferable to me.

Do you have a scenario where an exception is thrown rather than a warning?

mfb’s picture

mfb
they or he
commented
StatusFileSize
new 3276224-TEST-ONLY.patch635 bytes
mfb’s picture

mfb
they or he
commented
StatusFileSize
new 3276224-6-TEST-ONLY.patch626 bytes

  • mfb committed b2912b1 on 2.x 
    Issue #3276224 by mfb: Test failed to open stream handling
mfb’s picture

mfb
they or he
commented
Issue tags: -Needs tests

I committed the test for current handling, but if we have scenarios where exception is thrown (maybe involving non-core code?) then we should expand the test coverage.

mfb’s picture

mfb
they or he
commented
Status: Postponed (maintainer needs more info) » Fixed

I'm tentatively marking this fixed as I added a test for the intended behavior, but please re-open if you have a scenario where an exception is being thrown instead of a warning being logged.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.