Ticket #12479 (closed Bug: fixed)

Opened 2 years ago

Last modified 2 years ago

Products.ResourceRegistry: "File" content lost (again)

Reported by: dieter Owned by: datakurre
Priority: major Milestone: 4.x
Component: General Version: 4.1
Keywords: patch, TuneUp54 Cc: dieter@…

Description

I have hit now another problem similar to that described in #9849 and #11583. I have "kupudrawers.js" customized. This gives an "OFS.Image.File" object using a nested "PData" structure to maintain its file content (as "kupudrawers.js" is sufficiently large). The "index_html" of such an object uses "Response.write" to "deliver" the content and heavily confuses the resource registry.

The resulting behavior is extremely nasty: the browser gets the customized content in a perfectly looking response, probably followed by junk (the remaining content of a concatenated resource). However, the content of the concatenated resource without the customized file content is stored in the RAM cache and delivered on subsequent requests.

The bug affects all "FSFile" customizations for resources with a sufficiently large content (such that "File" uses "Response.write" rather than return the content directly as a string).

The bug is in all versions of "Products.ResourceRegistries" up to 2.1a1.

Attachments

ResourceRegistries_12479.patch Download (1.5 KB) - added by dieter 2 years ago.
patch (against version 1.5.7)

Change History

Changed 2 years ago by dieter

patch (against version 1.5.7)

comment:1 Changed 2 years ago by kleist

  • Milestone set to 4.x

comment:2 Changed 2 years ago by jonstahl

  • Keywords patch added
  • Version set to 4.1
  • severity set to Untriaged

comment:3 Changed 2 years ago by eleddy

  • severity changed from Untriaged to Normal

comment:4 follow-up: ↓ 6 Changed 2 years ago by datakurre

  • Owner set to datakurre
  • Status changed from new to assigned

I think I've both fixed and tested this a few months ago with a little different approach with

 https://github.com/plone/Products.ResourceRegistries/commit/da31f9a01863a28a3dea70f3ff0defe933f4be26

in

 https://github.com/plone/Products.ResourceRegistries/tree/datakurre-ofs-file-pdata

@dieter, could you please confirm that my branch fixes this for you also? If so, I could make a pull request out of it and get this patch merged.

comment:5 Changed 2 years ago by datakurre

  • Keywords patchTuneUp54 added; patch removed
  • Status changed from assigned to confirmed

comment:6 in reply to: ↑ 4 Changed 2 years ago by dieter

Replying to datakurre:

I think I've both fixed and tested this a few months ago with a little different approach with

 https://github.com/plone/Products.ResourceRegistries/commit/da31f9a01863a28a3dea70f3ff0defe933f4be26

in

 https://github.com/plone/Products.ResourceRegistries/tree/datakurre-ofs-file-pdata

@dieter, could you please confirm that my branch fixes this for you also? If so, I could make a pull request out of it and get this patch merged.

The fix solves the concrete problem I have observed. However, it is far from robust. For example, for a former employer, I have changed the customization type for "FSFile" from "File" to "CMFAwareFile" (such that the customized object is as "caching_policy_manager" aware as the original). "CMFAwareFile" would have the same problem as "File" (with respect to this bug), but would not be handled by your special case "meta_type" based handling. The same applies to all other "File" derived classes used as a skin object.

comment:7 Changed 2 years ago by datakurre

  • Status changed from confirmed to assigned

Thanks. I see, your approach is more robust in that sense and I should go your way in that.

comment:8 Changed 2 years ago by datakurre

  • Status changed from assigned to confirmed

comment:9 Changed 2 years ago by datakurre

  • Keywords patch, TuneUp54 added; patchTuneUp54 removed

comment:10 Changed 2 years ago by datakurre

Ok. Refactored. All tests seem to pass. Created a pull requests for 2.0 branch.

 https://github.com/plone/Products.ResourceRegistries/pull/4

comment:11 Changed 2 years ago by datakurre

  • Status changed from confirmed to closed
  • Resolution set to fixed

comment:12 Changed 22 months ago by davisagli

  • Component changed from Infrastructure to General
Note: See TracTickets for help on using tickets.