Embeded ressources may break when editing phpliteadmin.php

Issue #375 new
T. Todua created an issue

Hi Developers of PLA. Excellent work. Just I had a problem (I opened an issue too) which is caused by quite strange approach you used in PLA. I am not a guru of PHP, however, I haven't anywhere seen using embedded resources this way in PLA uses. And obviously (in addition to the fact that this is hard, complex, and non-standard approach), this makes problems to users like me (like in the mentioned issue I experienced). I've made (in dev version) an update, please review those changes (only in bottom, after output function), which I am sure, is better, and merge in your project if you can. thanks in advance.

Comments (12)

  1. phpLiteAdmin repo owner

    Hi. Thanks for your feedback. Please have a look at our repository. You won't find a phpliteadmin.php in there. We distribute PLA as a single file phpliteadmin.php for simple deployment. But we don't develop it like this. In the repository, you find many source files that we use for development. In there, scripts, css and images are separate files. You will also find a build.php script and this script produces the single file phpliteamin.php.

    We cannot use diffs against phpliteadmin.php (especially the totally generated part at the bottom), as this is a generated file. If you want to propose a change, it would need to be based on the files in the repository. If you want to propose changes regarding the embedded ressources at the bottom, this most likely needs to be a change of the generator (build.php).

    I will later have a deeper look at what your diff proposes. But I guess it partly breaks the way we develop PLA. Because PLA is not only able to run as single-file. It also runs from many files by running the index.php from the repository. At first sight, some of your changes seem to break this behavior.

  2. T. Todua reporter

    Thanks for the response. Actually, do as you wish, just I've faced a real issue, the file doesnt work on my server, and I've pointed to the issue i've opened for that. And the fix I found, i've proposed.

    The end of story is that it should work and doesn't matter how you will develop that, but just my 2 cents, that it would be nice if the fix was made, whatever manner you want. (my thougth was what I've suggested, more readable and logical, than complex method it was before). thanks

  3. phpLiteAdmin repo owner

    Sure I will have a look at the problem that you faced and find a solution, maybe also based on your proposal. Thanks for reporting.

  4. phpLiteAdmin repo owner

    I can confirm the original issue #374. It can happen when the file phpliteadmin.php is edited with a texteditor that changes the newlines within the file. Currently the file has mostly CR+LF to make sure it can be edited with Windows Editor. However, the JavaScript embedded at the end seems to only have LF. Some editors will replace these LF with CR+LF to make the file consistent. When we load the javascript, we do it based on the bytes after the compiler halt position. As the newlines have changed, the javascript is now one byte longer for each line of JS. However, the ressource length is not adjusted. This is why we end up with an incomplete JavaScript (and a broken favicon!). I think the Javascript has only LF newlines because the javascript minifier we use when building removes the CRs.

    So I know where the problem comes from. Thanks a lot for making us aware. I will think about how to solve it best.

  5. T. Todua reporter

    thanks for noticing that. ok, whatever you prefer. just that method being used (with indexing and etc...) is quite inconsistent and would be nice if used standard and reliable way, like, like outputing them from function as the example i've given above. many thanks, When you will issue next update, i will update it on my site too.

  6. phpLiteAdmin repo owner

    The standard way of doing what we do (packaging everything in one file) would be to create a PHAR archive. However, most webservers are not configured to handle .phar-files and there are some PHP bugs (related to PHP FPM etc.).

    Agreed, your way is more readable and less error-prone than our current approach.

    I still need to think about it. Probably I will release a quick fix first that just keeps the CR LF in the JavaScript... Then we can take the time to evaluate different solutions.

  7. T. Todua reporter

    Ah , you're right about that, just I didn't mean that industrial standard way, which will much burden in this case:) I just meant something simpler alternative and "more accepted" way of doing that, like the suggested change.

    thanks for response, I'll not keep you longer in this chat, take care ! nice job!

  8. phpLiteAdmin repo owner

    In version 1.9.8, with commit b1062c89f953a795562a7cf355aa1a056966a639, I took the route to use CR+LF everywhere in order to avoid this problem.

    Still, we should think about our resources and how to embed them in a way that is not so fragile. Another way may be to use a CDN and load them from there.

    For those users that require PLA to work offline, we could offer a second resources-file that you can just download alongside PLA.

    We already load the SQL highlighter from a CDN, so at the moment, it is a bit of a mixture between CDN and embedded resources. Maybe we could embed all core CSS as it is strictly necessary and load all JS from CDN, as PLA still works without the JS.

  9. Log in to comment