Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.4+] IPP_OP_CREATE_LOCAL_PRINTER creates raw queue for model 'everywhere' if PPD generation fails #760

Closed
10ne1 opened this issue Jul 21, 2023 · 7 comments
Labels
bug Something isn't working priority-high

Comments

@10ne1
Copy link
Contributor

10ne1 commented Jul 21, 2023

Hi, I am upgrading CUPS from v2.3.3 to 2.4.6 in ChromiumOS where the regression is a bit more complex due to some downstream divergence, but I've reproduced the issue on my Arch Linux with unmodified CUPS from the Arch Linux Archive.

Below you have a minimal reproducing use case for both failures.

Describe the bug
Starting with this CUPS v2.4 commit, it appears certain parts of lpadmin were moved inside the scheduler to run in a background thread. This causes two specific regressions because the failures are not caught by lpadmin anymore (other failures like passing a bad ppd are still caught).

To Reproduce
A minimal reproducing use case follows for 2.3.3 vs 2.4.6:

On v2.3.3:

$ lpadmin -v ipp://localhost:631/bad_request -p NotAPrinter -m everywhere -E
lpadmin: Unable to query printer: The printer or class does not exist.

$ lpstat -v -p is empty

$ lpadmin -v ipp://localhost:9101/ipp/print -p UnreachablePrinter -m everywhere -E
lpadmin: Unable to connect to "localhost:9101": Host is down

$ lpstat -v -p is also empty

on v2.4.6:

$ lpadmin -v ipp://localhost:631/bad_request -p NotAPrinter -m everywhere -E
$ echo $?
0
$ lpstat -v -p
device for NotAPrinter: ipp://localhost:631/bad_request
printer NotAPrinter is idle.  enabled since Fri Jul 21 15:55:44 2023

$ lpadmin -v ipp://localhost:9101/ipp/print -p UnreachablePrinter -m everywhere -E
$ echo $?
0
$ lpstat -v -p
device for UnreachablePrinter: ipp://localhost:9101/ipp/print
printer UnreachablePrinter is idle.  enabled since Fri Jul 21 16:17:31 2023

Expected behavior
The expected behaviour is what happens on 2.3.3.

Seems like in v2.4.6 the error checking still happens, but inside the scheduler? I built CUPS with debug enabled and saw the scheduler fail while running the _ppdCreateFromIPP2() function from ppd-cache.c:

2023-07-20T16:00:04.007496Z WARNING cupsd[9501]: T002 16:00:04.007  cupsSendRequest(http=0xf09075b8, request=0xf0915768(Get-Printer-Attributes), resource="/bad_request", length=166)
2023-07-20T16:00:04.007127Z DEBUG cupsd[9505]: NotAPrinter: Get-Printer-Attributes returned 1030: client-error-not-found (Not Found)                                                         
2023-07-20T16:00:04.009063Z ERR cupsd[9505]: ipp://localhost:631/bad_request:  Failed to execute Get-Printer-Attributes request

Maybe there needs to be a method to propagate the error from the scheduler thread to lpadmin? Maybe some inter-process communication like a named pipe? Just throwing ideas out, what do you think?

System Information:
Latest Arch Linux as well as ChromeOS.

What I did for ChromeOS for now is revert the commit I mentioned above (quite the pain btw), to get the old lpadmin behaviour.

@zdohnal zdohnal changed the title IPP everywhere scheduler move regression [2.4+] IPP_OP_CREATE_LOCAL_PRINTER creates raw queue for model 'everywhere' if PPD generation fails Jul 24, 2023
@zdohnal
Copy link
Member

zdohnal commented Jul 24, 2023

Hi @10ne1 ,

thank you for reporting the issue - in general we discussed as part of #347 which is about side effect of the change (which you correctly found as the culprit) in case PPD generation takes too long, but it is good to have it as a separate tracker too.

@michaelrsweet is looking into implementation of solution for this, which we can use in 2.4 , so he might have some ideas in mind. I was thinking about this myself too and we might move IPP Get-Printer-Attributes from the separate thread into main thread - it would cause some slowdown for the server, but IMO it is better than creating printers which look as IPP Everywhere, but they are raw in the end.

P.S. @michaelrsweet imho the similar check whether the destination is capable of IPP Everywhere we will need for printer profiles creation for local server and permanent queues on sharing server.

@zdohnal zdohnal added bug Something isn't working priority-high labels Jul 24, 2023
@michaelrsweet
Copy link
Member

@zdohnal We absolutely don't want this happening in the main thread since it would be an easy way to implement a DoS attack - try adding a printer to a non-existent address and you prevent everyone from accessing the server for at least 30 seconds...

Ideally, the response logic for this operation should be updated to use the CUPS-Get-Devices/CUPS-Get-PPDs interface so that the response can be fed back to the client over a pipe once the PPD is or is not available.

@zdohnal
Copy link
Member

zdohnal commented Jul 24, 2023

@michaelrsweet IIUC pipes would work locally, but we support remote adding printers (lpadmin -h, a remote request for cupsd) - I would guess it won't work like this?
Only other thing I can think of is turning the server concurrent...

@michaelrsweet
Copy link
Member

@zdohnal The pipe is running locally to return the IPP response to the network client which can be local (domain socket or loopback) or remote (external network interface). Like I said, the same way we do the CGI-based cups-driverd and cups-deviced interfaces for CUPS-Get-PPDs and CUPS-Get-Devices.

@10ne1
Copy link
Contributor Author

10ne1 commented Mar 20, 2024

Hello, Gentle ping - has there been any progress on this or issue #347 ?

@zdohnal
Copy link
Member

zdohnal commented Mar 20, 2024

Not at the moment, but I would like to come with something for 2.4.8. Currently I'm stuck with different things.

@michaelrsweet
Copy link
Member

OK, changes for #347 should address the issues here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-high
Projects
None yet
Development

No branches or pull requests

3 participants