Viewing 6 replies - 1 through 6 (of 6 total)
  • Plugin Author David Lingren

    (@dglingren)

    Thank you for opening this topic to address the MIME Type issues you identified in your review of the Gedshow plugin. Thanks again for all the work you did there to investigate and document the problem and write up your results. I have responded to the points you raised below.

    I have uploaded a new MLA Development Version dated 20230309 that corrects the original problem. You can find step-by-step instructions for using the Development Version in this earlier topic:

    PHP Warning on media upload with Polylang

    Once the Development Version is installed you can retest the handling of the .ged MIME type and file extension added by the Gedshow plugin.

    The fix will be part of my next MLA version, but in the interim it would be great if you could install the Development Version and let me know if it works for you. Thanks for inspiring this MLA improvement.

    In your review comments you correctly identified MLA’s current shortcoming. MLA only looks for custom MIME types once, when it first builds out its own MIME type support. If you add a plugin such as Gedshow later, the type it tries to add is ignored. This was my original design decision and I mitigated it by allowing such types to be added manually. I have come to regret that decision over time, leading to this latest MLA improvement.

    I outlined the manual addition steps in the topic you found as well as this more recent topic:

    Unable to upload .ged file

    @colinsp – I invite you to give the Development Version a try and let me know if it works for you.

    In your review comments you wrote:

    Adding ‘ged’ as a MLA filetype still has problems because your next action on line 344 is $uploaded = media_handle_upload('ged_upload', 0);
    If I add it to MLA as David identifies, I get past the first error (which was, I believe, previously addressed in https://www.ads-software.com/support/topic/trouble-with-a-ged-upload/) and I get the new error ‘Error uploading file: Sorry, you are not allowed to upload this file type.‘ from line 344. The reason for this is that media_handle_upload() calls wp_handle_upload() which again checks for a valid mime type, but differently since MLA with a ged mime-type gets past the 337/338 check but not the 344 line.

    I was not able to reproduce this problem in my testing; adding the type manually worked for me. I have a very small GedCOM file to test with, and it is possible that your failure is related to the content of your file. You wrote “I suggest you may want to look at the detail docs for _wp_handle_upload to see if you are hooking all of the mime-type verification hooks.” The wp_handle_upload() function calls wp_check_filetype_and_ext() to validate the MIME type. That function has trouble with some file content and sometimes returns the wrong MIME type. For example, a CSV file that contains HTML markup will be returned as text/html, not text/csv. This is why I added an “Always Use MLA MIME Type” option to the Settings/Media Library Assistant Uploads tab. You can try checking that box and see if it solves the issue with your file.

    You added “I hate WordPress. It’s programming in a totally chaotic environment.” I don’t hate WordPress but there are parts I really dislike. WordPress handling of MIME types is quirky and murky at best, which is how MLA’s Uploads processing was born. It’s not my favorite part of WordPress, either.

    Thread Starter joevansteen

    (@joevansteen)

    @dglingren

    Using your new version, which includes a ‘ged’ entry in your internal tables, the GedShow program seemed to work for me. However,

    • trying to override your ged encoding with my own, making the mime type ?application/octetstream (potentially the correct encoding) I again get a fail.
    • making your ‘ged’ type inactive, since I could not delete it, to mimic the case where a mime-type unknown to your program is attempted to be added by someone other than you, again fails.

    Just changing your coding from plain text to octet isn’t really the issue for me. I don’t know how or why you think that is the right way to go. You should allow any other program to handle any mime-type they want, and still be able to use the WP filters in the manner the GedShow plugin was trying to do. I have other mime-types unknown to you in my media library and you ignore them just fine, but the code for those types is accomplished in plugins which don’t use the same WP filters and hooks. I haven’t tried to debug every one of them, but just as I mentioned to the GedShow author, RootsPersona had no trouble with ‘ged’ as a filetype to upload because they simply didn’t depend on WP filter handling to process the file. For anyone that does, like GedShow, your plugin breaks their code. That second case above, IMHO, is the bigger problem.

    Why did you think you needed to add a ‘ged’ type? You are taking over the responsibility as the authority for any and all mime-type that utilize the WP mime filters. That ain’t friendly, and it isn’t the way I understand WP is supposed to work.

    I would assume that if you remove the ‘ged’ type from your own internal tables your test should fail the same as mine. That should be your test to see if you’ve made your code accommodating for others.

    Plugin Author David Lingren

    (@dglingren)

    Thanks for taking the time to install and try the Development Version. Thanks as well for the detailed report on your experience.

    Regarding the selection of MIME type you wrote “making the mime type application/octet–stream (potentially the correct encoding)“. The official GEDCOM 5.5.5 Specification says:

    The specification now explicitly states that GEDCOM files are text files, and that while GEDCOM allows multiple line terminators, each GEDCOM file must use a single line terminator choice throughout the file.

    Given that, text/plain seems a reasonable choice, and is the choice made by the Gedshow plugin.

    That said, I wanted to reproduce your trouble with the MIME type you tried to set. I copied the type from your post and pasted it into the Edit Upload MIME Type form. On clicking “Update” I got this message ‘ERROR: Bad MIME type; try “application/octetstream” ‘; is that what you saw? It turns out that the “dash” in your post is not a plain dash, it’s an EM dash as shown in this hex dump:

    application/octe 61 70 70 6c 69 63 61 74 69 6f 6e 2f 6f 63 74 65
    t...stream       74 e2 80 93 73 74 72 65 61 6d
    

    I changed it to application/octet-stream with a plain dash and the error was resolved. Of course, this could be an artifact of your post; if your problem was different, let me know.

    You wrote “making your ‘ged’ type inactive, since I could not delete it, to mimic the case where a mime-type unknown to your program is attempted to be added by someone other than you, again fails.” After disabling the Gedshow plugin I was able to delete the entry. With Gedshow enabled, the deletion works and is immediately followed by re-discovering the type added by Gedshow, adding it back to MLA’s list. If your problem was different, let me know.

    Making a type known to MLA “inactive” is not the same as deleting it. As the Edit Upload MIME Type screen says, “Check this box if you want to remove this entry from the list of Upload MIME Types returned by get_allowed_mime_types().” In other words, inactive means “I know this MIME Type and I don’t want it in the list.” Its main use is to preserve the definition of a custom type for possible future use, or to explicitly prevent uploading a type that would otherwise be accepted.

    I have tried to make MLA as WordPress-like as possible, using WordPress api functions and hooks wherever possible. I am suspicious of themes and plugins where “the code for those types is accomplished in plugins which don’t use the same WP filters and hooks.“, but that is my personal opinion. You wrote “For anyone that does, like GedShow, your plugin breaks their code. That second case above, IMHO, is the bigger problem.” In my testing on my system I see no evidence of that. If I can reproduce a problem I will fix it.

    You wrote “Why did you think you needed to add a ‘ged’ type?” I added .ged to my optional list of known types, but unless an MLA user adds it manually it is the Gedshow plugin (in this case) that is adding it. Once added by Gedshow it is retained in MLA’s “internal tables” so that, for example, the user can assign a different thumbnail icon to display in the Media Library and Media/Assistant tables.

    Using MLA’s MIME Type is optional. If it causes trouble or is simply not wanted, just uncheck the “Enable Upload MIME Type Support” box and click “Save Changes”.

    If I have not understood or reproduced any of your issues accurately, any clarification or additional details will help me investigate further. Thanks for your understanding, your patience and your help.

    Thread Starter joevansteen

    (@joevansteen)

    @dglingren I apologize and stand corrected on the correct mime type for GEDCOM files, it is text/plain.

    I’m also going to close this because it is using too much of my time. I’m caught here in the middle of a problem between two plugins which I’ve tried to use, neither of which is mine. Trying to reproduce errors that emanate from an attempt to artificially re-create problems in not an effective use of my time, or yours.

    When looking at your new code, you still appear to be registering your mime_types filter with the highest priority making it the last filter used by WP. (Something which should be unnecessary unless you have a very real need to do so.) And, you (still) seem to be replacing rather than appending the mime types array supplied as an input parameter to the filter. (Possibly related to why you found it necessary to use the high priority.) Am I wrong on these points? If so, again, I apologize for misreading your code.

    If I am not wrong, that coding strategy, IMHO, is a problem for anyone attempting to use their own use of the filter to supply a mime type of a variety unknown to your program. That, IMHO, would be a problem with integrating MLA with a program that might want to dynamically define those other types unknown to MLA. Such as ‘atom’, ‘ecma’, epub’, ‘json’ or a host of others.

    Thanks for bearing with me, but I really do have other fish to fry.

    Thread Starter joevansteen

    (@joevansteen)

    @dglingren On re-reading of your last comment you seem to be implying that any mime type added by another program dynamically will be incorporated into your own table. At some point perhaps I’ll try to find the time to test that.

    I appreciate your plugin and your attempt to make this right; but spending more time on it isn’t my highest priority.

    Plugin Author David Lingren

    (@dglingren)

    Thank you again for taking the time to engage in this thoughtful dialog about MLA’s MIME Type handling. I understand your desire to move on to other priorities. Let me address some of your final comments.

    You wrote “On re-reading of your last comment you seem to be implying that any mime type added by another program dynamically will be incorporated into your own table.” Yes, that’s right. This was the original issue raised (with Gedshow) and is fixed in the Development Version. It will be part of my next MLA version.

    You wrote “you still appear to be registering your?mime_types?filter with the highest priority making it the last filter used by WP.” Yes, that has always been the case. MLA runs last so I can be sure to discover and custom MIME Types added by themes and plugins.

    You wrote “And, you (still) seem to be replacing rather than appending the mime types array supplied as an input parameter to the filter.” Yes, that’s true. It ensures that any changes a user makes in the MLA “Uploads” tab is reflected in the MIME Types passed back from the filter. For example, a user could decide to change the MIME Type assigned to a particular file extension.

    You wrote “that coding strategy, IMHO, is a problem for anyone attempting to use their own use of the filter to supply a mime type of a variety unknown to your program.” I believe that problem is fixed in the Development Version and my next MLA version.

    I am leaving this topic resolved, but please update it if you have any further thoughts. Thanks for working with me.

Viewing 6 replies - 1 through 6 (of 6 total)
  • The topic ‘Mime type handling’ is closed to new replies.