Not able to upload any files using @theming-controlpanel-mapper

[NicolasGoeddel]

There is an issue with uploading files when plone.rest is installed. I always get a 404 error when trying to upload a file. But I am able to create an empty file an copy paste the content to it.

The guys from plone.rest told me it's your fault because themefile.py is using "application/json" responses every time instead of the proper status codes 200 and 422 (unprocessable entity).

Please can you fix that issue so we are able to upload files again?

At the moment we are using plone.app.theming == 4.0.1, plone.rest == 1.4.0 and plone.restapi == 4.3.1.

[djay]

This is incorrect. The problem is caused as plone.rest won't allow any other part of plone to use apis that expect JSON as a response and expect all other parts of plone to not expect JSON responses.

[NicolasGoeddel]

I only want to make it work again. I don't know what's the best way to do it. I don't care who did what wrong. ;-) I hope you guys will find a solution.

[djay]

There is already a working fix. plone/plone.rest#61. We use it in production.

[ewohnlich]

I've not been able to implement this. I have the following monkey patch and I verified with a debug trace that it was hitting the expected condition and return obj, but it doesn't actually put anything in portal_resources

    try:
        obj = super(RESTTraverse, self).publishTraverse(request, name)
        if not IContentish.providedBy(obj) and not IService.providedBy(obj):
            if isinstance(obj, VirtualHostMonster):
                return obj
            elif isinstance(obj, BTreeFolder2) and obj.id == 'portal_resources':
                return obj
            else:
                raise KeyError
    except KeyError:
        return self._
old_restPublishTraverse(request, name)```

I get a 200 response back in the browser

[tisto]

Just for the record. This method in plone.app.theming is causing the problem:

https://github.com/plone/plone.app.theming/blob/master/src/plone/app/theming/browser/themefile.py#L25

The call method clearly violates HTTP/REST standards and best practices because:

• it always returns the HTTP status code 200 (it should return 3xx if an error occurred -> HTTP standard violation)
• it always returns a body (if you send 200 you should not send a body (otherwise use 204) -> HTTP standard violation)
• it encapsulates the error/success message in the HTTP body (this is what HTTP status codes are for -> HTTP standard violation)

If this function would be implemented properly without violating the core of the HTTP standard, no JSON would need to be sent over the wire and no HTTP accept header "application/json" would need to be sent.

[petri]

The front-end code that sets the "application/json" accept header for the upload request is not in this package afaik. Which package has the frontend? Some pattern, any idea which?

[ewohnlich]

The mockup package calls the BrowserView mentioned here ("themeFileUpload") in the filemanager pattern, if that's what you mean.

[tisto]

If you do not want to refactor the code too much you can just use a different Accept header, e.g. rename "application/json" to "application/json+whatever". This is a two lines change that will fix the issue. I'd recommend fixing this properly of course but that would be a quick fix.

[petri]

Oh well. I could not find where mockup code sets the accept: header. I'm starting to think maybe it's set by default by some frontend library that patterns use (e.g. dropzone.js) ...

[tisto]

I never used mockup or patternslib, so I have no clue how this works.

[petri]

Taking a bit closer look at the patterns/mockup theme editor frontend, "violation of HTTP/REST standards and best practices", as @tisto put it, seems pretty widespread. Many of the theme editor TTW operations seem to use returned JSON success/error values, anyway.

So improving purity of just upload feature here & in frontend might leave the theming system in inconsistent state - things'd work differently per operation (e.g. upload vs. rename vs. whatever). Unless of course all those other operations were changed as well, including what seems quite a few tests.

As for changing the Accept: header, I found out it is indeed currently set to application/json as a default in the underlying Dropzone.uploadFiles prototype method. So in addition to changing things on the server side, here, that default would need to be overriden in the frontend pattern(s).

[petri]

The relevant code in Dropzone.js (current master) can be found in its _uploadData method . I first thought there'd be no other way to change the header than fork the library or monkey-patch the whole 94-line method. But upon closer look, it does allow overriding the default headers. So it seems to be possible to change both the mockup pattern + plone.app.theming so that uploading could work again despite plone.rest.

However, as I think @djay mentioned somewhere, if the Accept: header is just bluntly overriden by the pattern, it affects all use of the pattern in Plone. We'd have to extend or instrument the pattern so that you could override the default hardcoded header just in cases when necessary (removing it completely is not possible without major monkey-patching, or forking).

While it seems possible to override the header from the filemanager pattern, to cut a long story short, fixing this issue in the pattern + plone.app.theming is hardly an easy quickfix. On the contrary.

Whereas it'd be just one more three-line simple quickfix in plone.rest.

[petri]

see plone/plone.rest#105 that would fix this issue.