Skip to content

Conversation

@tacoberu
Copy link
Contributor

  • bug fix? yes
  • BC break? no

If the file has strict privileges, it will crash the entire application.

throw new Nette\Application\BadRequestException("File '$file' doesn't exist.");
}
if (!is_readable($file)) {
throw new Nette\Application\BadRequestException("File '$file' doesn't readable.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is not readable sounds better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how useful the is_readable check is when you check fopen result later anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JanTvrdik The sooner the better :-) But I don't insist on it.

$handle = fopen($this->file, 'r');
if (!$handle) {
throw new Nette\Application\BadRequestException("Cannot make handle of '{$this->file}'.");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot open file?

@dg dg force-pushed the master branch 4 times, most recently from 7fe78c1 to 8f1bb54 Compare December 11, 2019 19:32
@dg dg changed the title Fix FileResponse FileResponse: added some checks Jan 3, 2020
@dg dg merged commit 54b0b5c into nette:master Jan 3, 2020
dg pushed a commit that referenced this pull request Jan 3, 2020
dg pushed a commit that referenced this pull request Jan 3, 2020
dg pushed a commit that referenced this pull request Jan 4, 2020
dg pushed a commit that referenced this pull request Jan 4, 2020
dg pushed a commit that referenced this pull request Jan 4, 2020
dg pushed a commit that referenced this pull request Jan 5, 2020
dg pushed a commit that referenced this pull request Jan 5, 2020
dg pushed a commit that referenced this pull request Jan 5, 2020
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.

3 participants