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.

Sign up for GitHub

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

Jump to bottom

Added the functionality of Preserved Comments in cupsd.conf when cups… #640

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Added the functionality of Preserved Comments in cupsd.conf when cups… #640

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4d13be6
Added the functionality of Preserved Comments in cupsd.conf when cups…
Ankit3002 Mar 22, 2023
b9c57d1
Done some changes in cupsFileGetConf() to preserve conmments in cupsd…
Ankit3002 Mar 22, 2023
ccf71a9
Added: Private Api to read comments and configuration from cupsd.conf
Ankit3002 Mar 29, 2023
d999b0d
Fixed : Coding Style issues
Ankit3002 Apr 1, 2023
1c0ae05
Updated: cupsd.conf.in
Ankit3002 Apr 3, 2023
d08c54f
Added: Support to read inline comments from cupsd.conf
Ankit3002 Apr 5, 2023
0a6b3b3
Simplified : _cupsFileGetConfAndComments function logic
Ankit3002 Apr 6, 2023
d351867
Fixed: Coding style issues
Ankit3002 Apr 13, 2023
d92236a
Fixed: Coding Style Issues
Ankit3002 Apr 13, 2023
f241386
Simplified: _cupsFileGetConfAndComments()
Ankit3002 Apr 16, 2023
952fd3f
Fixed: Missing handling of escaped #
Ankit3002 Apr 16, 2023
7470d73
Added: escaped # support
Ankit3002 Apr 17, 2023
d9c1986
Fixed: Coding Style Issues
Ankit3002 Apr 19, 2023
04b6730
Fixed coding style issues
Ankit3002 Apr 19, 2023
07e9450
Added : Private Api for Striping Inline Comment
Ankit3002 Jun 2, 2023
5150a47
Added : Debug Statement for cupsFileStripComment
Ankit3002 Jun 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions conf/cupsd.conf.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,35 @@ WebInterface @CUPS_WEBIF@
# Timeout after cupsd exits if idle (applied only if cupsd runs on-demand - with -l)
IdleExitTimeout @EXIT_TIMEOUT@

# Restrict access to the server...
# Access to the server root (/):
# - default action defined by 'Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
# - allowing access is required for printer sharing or remote administration
<Location />
Order allow,deny
</Location>

# Restrict access to the admin pages...
# Access to the admin pages:
# - default action defined by Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
<Location /admin>
AuthType Default
Require user @SYSTEM
Order allow,deny
</Location>

# Restrict access to configuration files...
# Access to the configuration files:
# - default action defined by Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
<Location /admin/conf>
AuthType Default
Require user @SYSTEM
Order allow,deny
</Location>

# Restrict access to log files...
# Access to the log files:
# - default action defined by Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
<Location /admin/log>
AuthType Default
Require user @SYSTEM
Expand Down
53 changes: 16 additions & 37 deletions cups/adminutil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ cupsAdminSetServerSettings(
cups_option_t *cupsd_settings, /* New settings */
*setting; /* Current setting */
_cups_globals_t *cg = _cupsGlobals(); /* Global data */
int linenum_check; /* same linenumber? */


/*
Expand Down Expand Up @@ -667,6 +668,7 @@ cupsAdminSetServerSettings(
* Copy the old file to the new, making changes along the way...
*/

linenum_check = 0;
cupsd_num_settings = 0;
in_admin_location = 0;
in_cancel_job = 0;
Expand Down Expand Up @@ -698,7 +700,7 @@ cupsAdminSetServerSettings(
if (server_port <= 0)
server_port = IPP_PORT;

while (cupsFileGetConf(cupsd, line, sizeof(line), &value, &linenum))
while (_cupsFileGetConfAndComments(cupsd, line, sizeof(line), &value, &linenum))
{
if ((!_cups_strcasecmp(line, "Port") || !_cups_strcasecmp(line, "Listen")) &&
(remote_admin >= 0 || remote_any >= 0 || share_printers >= 0))
Expand All @@ -709,7 +711,6 @@ cupsAdminSetServerSettings(

if (remote_admin > 0 || remote_any > 0 || share_printers > 0)
{
cupsFilePuts(temp, "# Allow remote access\n");
cupsFilePrintf(temp, "Port %d\n", server_port);
}
else
Expand Down Expand Up @@ -749,7 +750,6 @@ cupsAdminSetServerSettings(
localp = cupsGetOption("BrowseLocalProtocols", cupsd_num_settings,
cupsd_settings);

cupsFilePuts(temp, "# Share local printers on the local network.\n");
cupsFilePuts(temp, "Browsing On\n");

if (!localp)
Expand All @@ -774,8 +774,6 @@ cupsAdminSetServerSettings(

if (debug_logging)
{
cupsFilePuts(temp,
"# Show troubleshooting information in error_log.\n");
cupsFilePuts(temp, "LogLevel debug\n");
}
else
Expand Down Expand Up @@ -837,11 +835,6 @@ cupsAdminSetServerSettings(
{
wrote_admin_location = 1;

if (remote_admin)
cupsFilePuts(temp, " # Allow remote administration...\n");
else
cupsFilePuts(temp, " # Restrict access to the admin pages...\n");

cupsFilePuts(temp, " Order allow,deny\n");

if (remote_admin)
Expand All @@ -856,13 +849,6 @@ cupsAdminSetServerSettings(
{
wrote_conf_location = 1;

if (remote_admin)
cupsFilePuts(temp, " # Allow remote access to the configuration "
"files...\n");
else
cupsFilePuts(temp, " # Restrict access to the configuration "
"files...\n");

cupsFilePuts(temp, " Order allow,deny\n");

if (remote_admin)
Expand All @@ -877,13 +863,6 @@ cupsAdminSetServerSettings(
{
wrote_log_location = 1;

if (remote_admin)
cupsFilePuts(temp, " # Allow remote access to the log "
"files...\n");
else
cupsFilePuts(temp, " # Restrict access to the log "
"files...\n");

cupsFilePuts(temp, " Order allow,deny\n");

if (remote_admin)
Expand All @@ -899,18 +878,6 @@ cupsAdminSetServerSettings(
{
wrote_root_location = 1;

if (remote_admin > 0 && share_printers > 0)
cupsFilePuts(temp, " # Allow shared printing and remote "
"administration...\n");
else if (remote_admin > 0)
cupsFilePuts(temp, " # Allow remote administration...\n");
else if (share_printers > 0)
cupsFilePuts(temp, " # Allow shared printing...\n");
else if (remote_any > 0)
cupsFilePuts(temp, " # Allow remote access...\n");
else
cupsFilePuts(temp, " # Restrict access to the server...\n");

cupsFilePuts(temp, " Order allow,deny\n");

if (remote_admin > 0 || remote_any > 0 || share_printers > 0)
Expand Down Expand Up @@ -1058,7 +1025,19 @@ cupsAdminSetServerSettings(
cupsFilePrintf(temp, "%*s%s %s\n", indent, "", line, value);
}
else
cupsFilePrintf(temp, "%*s%s\n", indent, "", line);
{
if (strchr(line, '#') == NULL)
cupsFilePrintf(temp, "%*s%s\n", indent, "", line);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in the comment, I would let here the simple cupsFilePrintf() as it was there - the code will be simpler and we get rid of the new variable.

else
{
if(linenum == linenum_check+1)
cupsFilePrintf(temp, "%s\n", line);
else
cupsFilePrintf(temp, "\n%s\n", line);
linenum_check = linenum;
}
}

}

/*
Expand Down
136 changes: 136 additions & 0 deletions cups/file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,142 @@ cupsFileGetConf(cups_file_t *fp, /* I - CUPS file */
}


/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the function definition to an alphabetically correct place in the file (under _cupsFileCheckFilter())

* 'cupsFileGetConfAndComments()' - Get line and comments from a configuration file.
*/

char * /* O - Line read or @code NULL@ on end of file or error */
_cupsFileGetConfAndComments(cups_file_t *fp, /* I - CUPS file */
char *buf, /* O - String buffer */
size_t buflen, /* I - Size of string buffer */
char **value, /* O - Pointer to value */
int *linenum) /* IO - Current line number */
{
char *ptr; /* Pointer into line */


/*
* Range check input...
*/

DEBUG_printf(("2cupsFileGetConf(fp=%p, buf=%p, buflen=" CUPS_LLFMT
", value=%p, linenum=%p)", (void *)fp, (void *)buf, CUPS_LLCAST buflen, (void *)value, (void *)linenum));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remains of copy&paste :)


if (!fp || (fp->mode != 'r' && fp->mode != 's') ||
!buf || buflen < 2 || !value)
{
if (value)
*value = NULL;

return (NULL);
}

/*
* Read the next non-comment line...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy&paste remain...

*/

*value = NULL;

while (cupsFileGets(fp, buf, buflen))
{
(*linenum) ++;

/*
* Strip any comments...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy and paste leftover.

*/

if ((ptr = strchr(buf, '#')) != NULL)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with passing comments via line, we can detect the comment, then remove the leading and trailing whitespaces and return buf.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdohnal I made the changes as you requested. Now I 'm using line variable to get comments. Please review them.

if (ptr > buf && ptr[-1] == '\\')
{
// Unquote the #...
_cups_strcpy(ptr - 1, ptr);
}
else
{
// Strip the comment and any trailing whitespace...
while (ptr > buf)
{
if (!_cups_isspace(ptr[-1]))
break;

ptr --;
}

return (buf);
*ptr = '\0';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will become a dead code if we return before that.

Additionally I've realized we should deal with inline comments in case they appear (even though they are prohibited by cupsd.conf man page) in a way.

So I propose:

  • check whether there is text before '#':
    • if there is, strip the comment, since it is invalid and process further in the function and return the text before the '#' char
    • if there is only whitespace before '#', strip the leading whitespace and return the comment.

So the block if ((ptr = strchr(buf, '#')) != NULL) can become a handler for inline comments, which will strip the comment in case there is text before it, and the execution will process further.

}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally you should review every cupsFilePut() below in cupsAdminSetServerSettings() which add a comment - once we start supporting comments, we can't put new comments in.

Rephrase the comments mentioned in cupsFilePut() and put it to the proper place at conf/cupsd.conf.in, f.e. when line contains "" and we are in /admin location:

The code:

  if (remote_admin)
          cupsFilePuts(temp, "  # Allow remote administration...\n");
  else
          cupsFilePuts(temp, "  # Restrict access to the admin pages...\n");

The default cupsd.conf.in:

# Restrict access to the admin pages...
<Location /admin>
  AuthType Default
  Require user @SYSTEM
  Order allow,deny
</Location>

so once you remove the cupsFilePut(), update the cupsd.conf.in to look like this f.e.:

# Access to the admin pages:
# - default action defined by Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
<Location /admin>
  AuthType Default
  Require user @SYSTEM
  Order allow,deny
</Location>

Copy link
Author

@Ankit3002 Ankit3002 Mar 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else if (in_root_location && (remote_admin >= 0 || remote_any >= 0 || share_printers >= 0)) { wrote_root_location = 1;

if (remote_admin > 0 && share_printers > 0)
      cupsFilePuts(temp, "  # Allow shared printing and remote "
                     "administration...\n");
else if (remote_admin > 0)
      cupsFilePuts(temp, "  # Allow remote administration...\n");
else if (share_printers > 0)
      cupsFilePuts(temp, "  # Allow shared printing...\n");
else if (remote_any > 0)
      cupsFilePuts(temp, "  # Allow remote access...\n");
else
      cupsFilePuts(temp, "  # Restrict access to the server...\n");

    cupsFilePuts(temp, "  Order allow,deny\n");

if (remote_admin > 0 || remote_any > 0 || share_printers > 0)
{
  if (remote_any >= 0)
    cupsFilePrintf(temp, "  Allow %s\n", remote_any > 0 ? "all" : "@LOCAL");
  else
    cupsFilePrintf(temp, "  Allow %s\n", old_remote_any > 0 ? "all" : "@LOCAL");
}
  }`

@zdohnal for the above part what should be the comment that I need to write in conf/cupsd.conf.in ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example:

# Access to the server root (/):
# - default action defined by 'Order', see 'man cupsd.conf'
# - use 'Allow'/'Deny' for configuring access
# - allowing access is required for printer sharing or remote administration


/*
* Strip leading whitespace...
*/

for (ptr = buf; _cups_isspace(*ptr); ptr ++);

if (ptr > buf)
_cups_strcpy(buf, ptr);

/*
* See if there is anything left...
*/

if (buf[0])
{
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we could check whether the first char is # and if so, return buf. With appropriate comment.

* Yes, grab any value and return...
*/

for (ptr = buf; *ptr; ptr ++)
if (_cups_isspace(*ptr))
break;

if (*ptr)
{
/*
* Have a value, skip any other spaces...
*/

while (_cups_isspace(*ptr))
*ptr++ = '\0';

if (*ptr)
*value = ptr;

/*
* Strip trailing whitespace and > for lines that begin with <...
*/

ptr += strlen(ptr) - 1;

if (buf[0] == '<' && *ptr == '>')
*ptr-- = '\0';
else if (buf[0] == '<' && *ptr != '>')
{
/*
* Syntax error...
*/

*value = NULL;
return (buf);
}

while (ptr > *value && _cups_isspace(*ptr))
*ptr-- = '\0';
}

/*
* Return the line...
*/

return (buf);
}
}

return (NULL);
}


/*
* 'cupsFileGetLine()' - Get a CR and/or LF-terminated line that may
* contain binary data.
Expand Down
1 change: 1 addition & 0 deletions cups/file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ typedef struct _cups_file_s cups_file_t;/**** CUPS file type ****/
* Prototypes...
*/

extern char *_cupsFileGetConfAndComments(cups_file_t *fp, char *buf, size_t buflen, char **value, int *linenum) _CUPS_PRIVATE;
extern int cupsFileClose(cups_file_t *fp) _CUPS_API_1_2;
extern int cupsFileCompression(cups_file_t *fp) _CUPS_API_1_2;
extern int cupsFileEOF(cups_file_t *fp) _CUPS_API_1_2;
Expand Down