Greetings from the far future! No, the new model is sadly not here yet, but we have a shiny new UI :) Bumping this old thread since I`ve stumbled upon this issue recently and it took me a while to understand what the actual problem is (thanks Thomas!) and I feel it's something we really need to get fixed as soon as possible. Thomas was suggesting we add this on the list for the 7.x Roadmap and I`m obviously +1 for that. There is the matter of backwards compatibility, that we need to still address, as either to break it or keep it. My take would be to break it, since I consider it to be bad code to depend on this broken behavior in the first place and that huge security hole we keep dragging along needs to be plugged abruptly. WDYT? Thanks, Eduard On Fri, Sep 4, 2009 at 11:56 AM, Thomas Mortagne <[email protected]> wrote:
On Fri, Sep 4, 2009 at 10:39, Vincent Massol<[email protected]> wrote:
On Sep 4, 2009, at 10:30 AM, Thomas Mortagne wrote:
On Fri, Sep 4, 2009 at 10:07, Vincent Massol<[email protected]> wrote:
Hi,
Im trying to fix http://jira.xwiki.org/jira/browse/XWIKI-4274
Basically if you do $xwiki.getDocument("someDoc").getRenderedContent() it'll get executed in the context of the current doc which I believe is wrong especially since other signatures of getRenderedContent() execute in the target document's context.
I have fixed this locally but found that admin.vm for example is assuming that getRenderedContent() will get executed in the context of the calling doc (i.e. XWiki.Import when doing an import for example). FYI the chain flow is admin.vm -- getRenderedContent() --> XWiki.AdminSheet --> XWiki.AdminImportSheet --> importinline.vm, which requires the current doc to be XWiki.Import (to get/put attachments from/to it). I can fix this easily using a new getRenderedContent signature I've introduced.
However I'm wondering if we have other places that incorrectly use getRenderedContent() and assume it won't be rendered in the context of the target document.
Is this change too dangerous to make? If not know, we'll need to it quickly (2.1M1?) since it's an important bug IMO.
Yes it's a dangerous fix I think, getRenderedContent() is really not an hidden method so changing its behavior that much even if it's a fix is dangerous anyway.
But would you be ok to make this change in 2.1M1?
Or would you prefer we live with it and instead document that it doesn't execute in the document's contex (and instead add a new method that does)? It's much simpler for sure but keeps adding to the mess. We could decide to keep adding to the mess and fix it when we rewrite that part in some far future.
Actually i was mainly thinking that we will rewrite all that in the new model anyway so for me one way or another it's temporary code so i think i would be more in favor of not changing the behavior of the current getRenderedContent() and instead add new one for backward compatibility.
Thanks -Vincent
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs