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

PHP content type headers ignored? #1223

Closed
dennisnissle opened this issue Apr 10, 2024 · 5 comments
Closed

PHP content type headers ignored? #1223

dennisnissle opened this issue Apr 10, 2024 · 5 comments
Assignees
Labels
[Aspect] Browser [Type] Bug An existing feature does not function as intended

Comments

@dennisnissle
Copy link

Hi there,

I was just testing some functionality of my plugin which sends a PDF as inline to the browser:

header( 'Content-Type: application/pdf' );
header( 'Content-Disposition: inline; filename="test.pdf";' );

within the playground the headers seem to be ignored:
Bildschirmfoto 2024-04-10 um 15 50 17

I know that it used to work with playground once - but the current version doesn't seem to support it.

Best,
Dennis

@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Aspect] Browser labels Apr 12, 2024
@adamziel adamziel added this to the Zero Crashes milestone Apr 12, 2024
@adamziel
Copy link
Collaborator

Good catch @dennisnissle! I just added that to the project board as a high priority issue.

@bgrgicak bgrgicak self-assigned this Apr 23, 2024
@bgrgicak
Copy link
Collaborator

PDF was missing in the infer mime type list #1298 which prevented static PDF files from being opened with the PDF content-type.

I added it but PDFs still don't load inside the Playground iframe. I think this is related to how iframes work, but need to research it more.

This blueprint will open a PDF in Firefox, but in Chrome it will fail with status blocked:other. If you open the path in a new tag, the PDF will load.

So, I think that PHP-WASM works correctly here, but there is an issue in Playground caused by it running in an iframe.

I was able to get inline PDFs to work in Playground without adding the mime type.

<?php
$pdf = "%PDF-1.1\n\n";
$pdf .= "1 0 obj\n";
$pdf .= "<<\n";
$pdf .= "/Type /Catalog\n";
$pdf .= "/Outlines 2 0 R\n";
$pdf .= "/Pages 3 0 R\n";
$pdf .= ">>\n";
$pdf .= "endobj\n\n";

$pdf .= "2 0 obj\n";
$pdf .= "<<\n";
$pdf .= "/Type /Outlines\n";
$pdf .= "/Count 0\n";
$pdf .= ">>\n";
$pdf .= "endobj\n\n";

$pdf .= "3 0 obj\n";
$pdf .= "<<\n";
$pdf .= "/Type /Pages\n";
$pdf .= "/Kids [4 0 R]\n";
$pdf .= "/Count 1\n";
$pdf .= ">>\n";
$pdf .= "endobj\n\n";

$pdf .= "4 0 obj\n";
$pdf .= "<<\n";
$pdf .= "/Type /Page\n";
$pdf .= "/Parent 3 0 R\n";
$pdf .= "/MediaBox [0 0 595.28 841.89]\n";
$pdf .= "/Contents 5 0 R\n";
$pdf .= "/Resources << /Font << /F1 6 0 R >> >>\n";
$pdf .= ">>\n";
$pdf .= "endobj\n\n";

$pdf .= "5 0 obj\n";
$pdf .= "<< /Length 44 >>\n";
$pdf .= "stream\n";
$pdf .= "BT\n";
$pdf .= "70 50 TD\n";
$pdf .= "/F1 12 Tf\n";
$pdf .= "(Hello, world!) Tj\n";
$pdf .= "ET\n";
$pdf .= "endstream\n";
$pdf .= "endobj\n\n";

$pdf .= "6 0 obj\n";
$pdf .= "<<\n";
$pdf .= "/Type /Font\n";
$pdf .= "/Subtype /Type1\n";
$pdf .= "/BaseFont /Helvetica\n";
$pdf .= ">>\n";
$pdf .= "endobj\n\n";

$pdf .= "xref\n";
$pdf .= "0 7\n";
$pdf .= "0000000000 65535 f \n";
$pdf .= "0000000009 00000 n \n";
$pdf .= "0000000074 00000 n \n";
$pdf .= "0000000120 00000 n \n";
$pdf .= "0000000179 00000 n \n";
$pdf .= "0000000306 00000 n \n";
$pdf .= "0000000380 00000 n \n";
$pdf .= "trailer\n";
$pdf .= "<<\n";
$pdf .= "/Size 7\n";
$pdf .= "/Root 1 0 R\n";
$pdf .= ">>\n";
$pdf .= "startxref\n";
$pdf .= "492\n";
$pdf .= "%%EOF\n";

file_put_contents('/wordpress/dummy.pdf', $pdf);
header('Content-type: application/pdf');
header('Content-Disposition: inline; filename=dummy.pdf');
readfile('/wordpress/dummy.pdf');

@bgrgicak
Copy link
Collaborator

@dennisnissle could you please share more details on how to recreate your issue?

In my testing header('Content-type: application/pdf') is returned as expected (see example above).

@dennisnissle
Copy link
Author

dennisnissle commented Apr 23, 2024

@bgrgicak you are right - the issue was related to the (PHP) library I was using to inline/output the PDF. The library checks for the PHP_SAPI constant and does only output headers in case of a value differing from CLI. In case of the playground it does alway equal CLI - that's why the headers were missing. I'm now manually forcing the headers in a custom script.

PS: I was able to allow inline outputting the PDF (even in chrome) by removing the sandbox attributes of the iframe: https://github.com/WordPress/wordpress-playground/blob/trunk/packages/playground/remote/remote.html#L59

@bgrgicak
Copy link
Collaborator

PS: I was able to allow inline outputting the PDF (even in chrome) by removing the sandbox attributes of the iframe: https://github.com/WordPress/wordpress-playground/blob/trunk/packages/playground/remote/remote.html#L59

Thank you! This just saved me a lot of research 🙂

adamziel added a commit that referenced this issue Apr 25, 2024
Adds a PHP Process manager, a class that can spawn maintain a number of
PHP instances at the same time.

The idea is to have:

* A single "primary" PHP instance that's never killed – it contains the
reference filesystem used by all other PHP instances.
* A pool of disposable PHP instances that are spawned to handle a single
request and reaped immediately after.

Spawning new PHP instances is reasonably fast and can happen on demand,
there's no need to keep a pool of "warm" instances that occupies the
memory all the time.

Example usage:

```ts
const mgr = new PHPProcessManager({
	phpFactory: async () => NodePHP.load(RecommendedPHPVersion),
	maxPhpInstances: 4,
});

const instance = await mgr.getInstance();
await instance.php.run({ code });
instance.reap();

```

Related to #1287

## Remaining work

* Add a "context" parameter to `getInstance()` to enable setting the
correct SAPI Name, e.g. `CLI` or `FPM` depending on the purpose the PHP
instance is created for. This would solve issues like #1223.
Alternatively, the consuming program could call `setSapiName()` on the
PHP instance returned by `getInstance()` – but that assumes the factory
did not initialize the web runtime.

## Testing instructions

Confirm the unit tests pass. This PR only adds new code, it does not
plug in the PHPProcessManager class into any request dispatching code
yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Aspect] Browser [Type] Bug An existing feature does not function as intended
Projects
Archived in project
Development

No branches or pull requests

3 participants