Limit IPP Everywhere printer spool usage - #158
Conversation
| if (!reserve_job_data(job, (size_t)bytes)) | ||
| { | ||
| flush_document_data(client); | ||
| respond_ipp(client, IPP_STATUS_ERROR_NOT_POSSIBLE, "Document exceeds available spool space."); |
There was a problem hiding this comment.
Two things:
- Return status should be
IPP_STATUS_ERROR_TEMPORARY(server-error-temporary-error) because this is presumably a temporary issue
There was a problem hiding this comment.
You said two things but only listed one, which should be fixed now.
There was a problem hiding this comment.
Oops, I think I had something else in mind but never came back to this. :)
2d19bb6 to
40aacec
Compare
michaelrsweet
left a comment
michaelrsweet
left a comment
There was a problem hiding this comment.
This would be an interesting addition, however the limits need to be configurable. Also, either use a static limit based on the maximum filesystem capacity or a dynamic limit based on the available filesystem capacity.
| } | ||
| else if (bytes > 0 && !reserve_job_data(job, (size_t)bytes)) | ||
| { | ||
| respond_ipp(client, IPP_STATUS_ERROR_NOT_POSSIBLE, "Document exceeds available spool space."); |
| { | ||
| if (!reserve_job_data(job, (size_t)bytes)) | ||
| { | ||
| respond_ipp(client, IPP_STATUS_ERROR_NOT_POSSIBLE, "Document exceeds available spool space."); |
| // Maximum space used for job files in the spool directory. | ||
| // | ||
|
|
||
| #define IPPEVE_MAX_SPOOL_SIZE (1024 * 1024 * 1024) |
There was a problem hiding this comment.
If we are bothering to add this to ippeveprinter, let's make the limit and reservation values configurable.
| bool web_forms; // Enable web interface forms? | ||
| size_t urilen; // Length of printer URI | ||
| size_t urilen, // Length of printer URI | ||
| max_spool_size,// Maximum spool size |
There was a problem hiding this comment.
File sizes use off_t, not size_t. But if we want to keep this as close to IPP semantics as possible, perhaps we can just use int with units of k-octets (1024 bytes). This would also better account for filesystem overhead (block size, etc.)
| int cancel; // Non-zero when job canceled | ||
| char *filename; // Print file name | ||
| int fd; // Print file descriptor | ||
| size_t size; // Size of print file |
There was a problem hiding this comment.
Again, this needs to be off_t or just use int and store the size in k-octets ((size + 1023) / 1024)
| struct statvfs spoolinfo; // Spool filesystem information | ||
| double spoolfree; // Free space in spool filesystem | ||
|
|
||
| if (!statvfs(printer->directory, &spoolinfo)) |
There was a problem hiding this comment.
This code needs to support both statfs and statvfs, just like the original code. In addition, the f_bavail value is a snapshot of free space vs. the f_blocks which reflects the maximum available space. I think it would be better to report the maximum value or have a function that returns the current availability that is used to report "job-k-octets-supported" to clients.
Print-Job, Send-Document, Print-URI, and Send-URI stream document data into the spool directory without a cumulative limit. A remote client can retain completed job files and exhaust the filesystem. Track job-file space in k-octets and reject document data that exceeds the configured aggregate limit. Make the spool limit and free-space reserve configurable, with defaults of 1GiB and 100MiB respectively. Use statfs or statvfs to bound the advertised maximum by total filesystem capacity. Check the current free space before every write to preserve the configured reserve.
40aacec to
95d584f
Compare
|
Oh, also can you make sure your commits are GPG-signed? |
|
I don't have GPG signing configured at this time. |
2 participants
Print-Job, Send-Document, Print-URI, and Send-URI stream document data into
the spool directory without a cumulative bound. A remote client can retain
completed job files and exhaust the filesystem.
Limit aggregate job-file storage, reserve filesystem headroom on POSIX
systems, and advertise the enforced job-size maximum.