DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 02:28 --- (In reply to comment #10) I had a short look at the patch, and I have the following comments: Do not change the 0.93 documentation but the trunk documentation: src/documentation/content/xdocs/trunk/configuration.xml, etc. Oops, yes I missed that. I will update the patch file to make changes to the trunk xdocs and not 0.93. TargetResolution is a property of FOUserAgent because it may easily vary with each run. It is not a good idea if you move properties around. They are in FopFactory or in FOUserAgent for a reason. See http://xmlgraphics.apache.org/fop/trunk/embedding.html. There is no real change here. TargetResolution is still a property of FOUserAgent, it is just configured from FopFactory and propagated to the FOUserAgent. As before, the value of this property in FOUserAgent can be programmatically set at anytime during rendering. Although I did not (and will not) have enough time to look at the patch in detail, I have the feeling that it addresses more than user configuration validation. It does some refactoring. That requires careful inspection. It may look like quite a large patch, but there really isn't very much refactoring, just stricter value checking which addresses issues raised in bugs 40288 and 40120. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19540|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 02:42 --- Created an attachment (id=19545) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19545action=view) main patch file Who ever commits this patch will need to remember to update fop/status.xml with something resembling the following (remember to fill in the dev attribute in the action tag). Index: status.xml === --- status.xml (revision 504579) +++ status.xml (working copy) @@ -28,6 +28,9 @@ changes release version=FOP Trunk + action context=Code dev=?? type=fix fixes-bug=40120,40288 due-to=Adrian Cumiskey +Strict FOP user configuration checking. + /action action context=Code dev=JM type=add Support for GIF images in RTF output (RTF handler, only. Does not affect the RTF library.) /action -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 07:02 --- (In reply to comment #12) Created an attachment (id=19545) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19545action=view) [edit] main patch file I have another comment: this would be better to put the test config files in a subdirectory of test/ --typically, test/conf. Also, the tests fail with a SAXParseException: The processing instruction target matching [xX][mM][lL] is not allowed. Can aynyone else try and tell me if the problem also occurs? Finally, I got a small error applying your patch. I could recover easily, but next time be sure to create it from scratch and against the latest svn revision. Thanks, Vincent -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 02:28 --- (In reply to comment #10) I had a short look at the patch, and I have the following comments: Do not change the 0.93 documentation but the trunk documentation: src/documentation/content/xdocs/trunk/configuration.xml, etc. Oops, yes I missed that. I will update the patch file to make changes to the trunk xdocs and not 0.93. TargetResolution is a property of FOUserAgent because it may easily vary with each run. It is not a good idea if you move properties around. They are in FopFactory or in FOUserAgent for a reason. See http://xmlgraphics.apache.org/fop/trunk/embedding.html. There is no real change here. TargetResolution is still a property of FOUserAgent, it is just configured from FopFactory and propagated to the FOUserAgent. As before, the value of this property in FOUserAgent can be programmatically set at anytime during rendering. Yes, the patch doesn't seem to break anything. We could even go a bit further: the cfg parameter is no longer used in the FOUserAgent.configure() method, it might be removed. Also, would the FopFactory.getUserConfig() method not be public, I would even remove it, and move the logic that is in FOUserAgent.getUserRendererConfig into FopFactory. Is that getUserConfig() method supposed to have any utility to embedding applications? I would answer no, but what do embedding specialists think about it? Vincent
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 07:20 --- (In reply to comment #13) (In reply to comment #12) Created an attachment (id=19545) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19545action=view) [edit] [edit] main patch file I have another comment: this would be better to put the test config files in a subdirectory of test/ --typically, test/conf. The reason I didn't do this is because of concerns raised about this patch containing too much refactoring. But I am in agreement that it makes sense for the config files to live separately. So if nobody objects I will provide them in a separate zip attachment and move/remove the existing test/test.xconf to test/config/test.xconfig. Also, the tests fail with a SAXParseException: The processing instruction target matching [xX][mM][lL] is not allowed. Can aynyone else try and tell me if the problem also occurs? Do you have the stack trace for this? Finally, I got a small error applying your patch. I could recover easily, but next time be sure to create it from scratch and against the latest svn revision. Thanks, Vincent Sorry about this, I will try to be mindful of this in the future. Ideally I would do this everytime, but there have been a number of revisions to this patch and preparing a new patch from scratch every half day is not a good use of time. Its not always practically quick and easy to do as I am in parallel cracking on with other FOP development work. regards, Adrian. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
Vincent Hennebert wrote: Yes, the patch doesn't seem to break anything. We could even go a bit further: the cfg parameter is no longer used in the FOUserAgent.configure() method, it might be removed. Also, would the FopFactory.getUserConfig() method not be public, I would even remove it, and move the logic that is in FOUserAgent.getUserRendererConfig into FopFactory. Is that getUserConfig() method supposed to have any utility to embedding applications? I would answer no, but what do embedding specialists think about it? Vincent I had considered all these points when developing the patch, and I whole heartedly agree with them, they are all good ones :-).. but I wanted to try and be as minimal as possible in my refactoring - at the end of the day this should just be a fairly simple, straight forward patch after all. Making these sort of changes makes committer checking more difficult and time consuming. If you are happy to review my patch file with the above changes I am *more than happy* to make them :-). Going further along this train of thought.. IMO, in an ideal world all this config stuff should be abstracted in a separate class such as FopConfig - so code dependencies on the avalon Configurable interface would be minimised and configuration would be centrally managed. regards, Adrian.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 07:40 --- (In reply to comment #14) (In reply to comment #13) (In reply to comment #12) Created an attachment (id=19545) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19545action=view) [edit] [edit] [edit] main patch file I have another comment: this would be better to put the test config files in a subdirectory of test/ --typically, test/conf. The reason I didn't do this is because of concerns raised about this patch containing too much refactoring. But I am in agreement that it makes sense for the config files to live separately. So if nobody objects I will provide them in a separate zip attachment and move/remove the existing test/test.xconf to test/config/test.xconfig. Well, the existing test.xconf might be used for other purposes. AFAIU its purpose its not to test that the handling of user config files is correct, but only to configure other tests. So I would leave it as is and put in the conf/ subdirectory only files meant as tests for the config handling code. Also, the tests fail with a SAXParseException: The processing instruction target matching [xX][mM][lL] is not allowed. Can aynyone else try and tell me if the problem also occurs? Do you have the stack trace for this? Of course, sorry, here it is: at org.apache.fop.config.BaseUserConfigTestCase.testUserConfig(BaseUserConfigTestCase.java:6 8) Same error message for all of the testcases. It's a bit weird, I'm not even sure to understand it. Finally, I got a small error applying your patch. I could recover easily, but next time be sure to create it from scratch and against the latest svn revision. Thanks, Vincent Sorry about this, I will try to be mindful of this in the future. Ideally I would do this everytime, but there have been a number of revisions to this patch and preparing a new patch from scratch every half day is not a good use of time. Its not always practically quick and easy to do as I am in parallel cracking on with other FOP development work. I can understand. You may want to work on several copies of the Trunk for each different task. That way it will be very easy to generate new patches. Thanks, Vincent -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19545|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 08:19 --- Created an attachment (id=19548) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19548action=view) main patch file Contains suggested ammendments from Vincent. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19514|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 08:21 --- (From update of attachment 19514) this is now included in the main patch file -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41514] - [PATCH] Strict url validation of user config
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41514. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41514 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19493|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-02-08 08:27 --- (From update of attachment 19493) this is now contain in the main patch file -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41572] New: - Error in url(...) URI processing
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=41572.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=41572
Summary: Error in url(...) URI processing
Product: Fop
Version: 0.93
Platform: Other
OS/Version: other
Status: NEW
Severity: normal
Priority: P2
Component: fo tree
AssignedTo: fop-dev@xmlgraphics.apache.org
ReportedBy: [EMAIL PROTECTED]
URISpecification.getURL() cannot handle URIs like url('(foo)bar'). It will
extract (foo only.
The problem can be fixed by replacing
href = href.substring(4, href.indexOf())).trim();
with
href = href.substring(4, href.lastIndexOf())).trim();
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
FOP as performance test case for Apache Harmony
I've just stumbled over an interesting page: http://harmony.apache.org/performance.html FOP seems to be used to track the performance development of Harmony. Jeremias Maerki
