Bug 97091 - libspectre does not handle correctly when a FAKE %%EOF in the middle of a ps document
Summary: libspectre does not handle correctly when a FAKE %%EOF in the middle of a ps ...
Status: RESOLVED MOVED
Alias: None
Product: libspectre
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Carlos Garcia Campos
QA Contact: Carlos Garcia Campos
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-27 05:38 UTC by Flex Liu
Modified: 2018-10-12 21:36 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
the ps document include a FAKE EOF comment in the middle of the file. (38.80 KB, application/postscript)
2016-07-27 05:38 UTC, Flex Liu
Details
[PATCH] Added spectre_document_load_scanstyle to allow using a particular scanstyle instead of a hardcoded one (4.33 KB, patch)
2016-08-24 09:04 UTC, Antonio Larrosa
Details | Splinter Review

Description Flex Liu 2016-07-27 05:38:19 UTC
Created attachment 125346 [details]
the ps document include a FAKE EOF comment in the middle of the file.

I have some old ps documents been created by gle 3.3h, it include a FAKE EOF comment in the middle of the file, I found libspectre could not handle this correctly, it cause evince or okular doesn't display the ps document correctly.

I found these code in libspectre:
libspectre/ps.c
401     int respect_eof;            /* Derived from the scanstyle argument.
 402                                    If set to 0 EOF comments will be ignored,
 403                                    if set to 1 they will be taken seriously.
 404                                    Purpose; Out there are many documents which 
 405                                    include other DSC conforming documents without
 406                                    without enclosing them by 'BeginDocument' and
 407                                    'EndDocument' comments. This may cause fake EOF 
 408                                    comments to appear in the body of a page.
 409                                    Similarly, if respect_eof is set to false
 410                                    'Trailer' comments are ignored except of the
 411                                    last one found in the document.
 412                                 */
 413     int ignore_dsc;             /* Derived from scanstyle.
 414                                    If set the document structure will be ignored.
 415                                 */
 416     BEGINMESSAGE(psscan)
 417 
 418     respect_eof = (scanstyle & SCANSTYLE_IGNORE_EOF) ? 0 : 1;
 419     ignore_dsc = (scanstyle & SCANSTYLE_IGNORE_DSC) ? 1 : 0;

libspectre/ps.h
150 #define SCANSTYLE_NORMAL     0
151 #define SCANSTYLE_IGNORE_EOF (1<<0)
152 #define SCANSTYLE_IGNORE_DSC (1<<1)

So, why the default value of SCANSTYLE_NORMAL is 0, it cause respect_eof = 1, the FAKE EOF comment will interrupt the document. is it any compatibility issue here? thanks.
Comment 1 Petr Tesarik 2016-08-15 11:17:33 UTC
FWIW I don't think it's a fake EOF. It is part of an embedded EPS file. The specification is not very clear about handling such %%EOF marks in embedded documents.

Anyway, the actual question is not only why the default scanstyle honours these EOF marks, but also why the libspectre API offers no way to override that default from the application.
Comment 2 Antonio Larrosa 2016-08-24 09:04:31 UTC
Created attachment 125992 [details] [review]
[PATCH] Added spectre_document_load_scanstyle to allow using a particular scanstyle instead of a hardcoded one

This patch fixes the issue as Flex Liu suggested, and also adds a convenient function to allow library users to choose the scanstyle they want to use.
Comment 3 Carlos Garcia Campos 2016-09-03 07:10:35 UTC
Comment on attachment 125992 [details] [review]
[PATCH] Added spectre_document_load_scanstyle to allow using a particular scanstyle instead of a hardcoded one

Review of attachment 125992 [details] [review]:
-----------------------------------------------------------------

Thanks for the patch, I'm sorry for the delay to review it. I'm fine with exposing a way to modify the scan style, but not exactly this way, see my comments.

::: libspectre/spectre-document.c
@@ +54,4 @@
>  spectre_document_load (SpectreDocument *document,
>  		       const char      *filename)
>  {
> +   spectre_document_load_scanstyle (document, filename, SCANSTYLE_IGNORE_EOF);

This breaks backwards compatibility. We should keep using SCANSTYLE_NORMAL to ensure the same behavior, and apps should use the new function if they want a different behavior.

@@ +57,5 @@
> +   spectre_document_load_scanstyle (document, filename, SCANSTYLE_IGNORE_EOF);
> +}
> +
> +void
> +spectre_document_load_scanstyle (SpectreDocument *document,

Instead of adding load_scanstyle, I would add something more generic like load_with_options() that would allow us to expand it with more options in the future if needed, even if those options are not related to the scan style.

@@ +59,5 @@
> +
> +void
> +spectre_document_load_scanstyle (SpectreDocument *document,
> +		       const char      *filename,
> +		       int              scanstyle)

I think we should use an enum type for this, SpectreLoadOptions for example.

@@ +75,4 @@
>  		document->doc = NULL;
>  	}
>  	
> +	document->doc = psscan (filename, scanstyle);

And extract the scanstyle from the option flags, instead of exposing SCANSTYLE macros in the public API.

::: libspectre/spectre-macros.h
@@ +45,5 @@
>  #endif
>  
> +#define SCANSTYLE_NORMAL     0
> +#define SCANSTYLE_IGNORE_EOF (1<<0)
> +#define SCANSTYLE_IGNORE_DSC (1<<1)

I don't think this belongs here. This should be defined in spectre-document.h and as an enum type.
Comment 4 GitLab Migration User 2018-10-12 21:36:58 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/libspectre/libspectre/issues/21.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.