Author: jsmeix Date: Wed Nov 5 14:00:50 2008 New Revision: 52923 URL: http://svn.opensuse.org/viewcvs/yast?rev=52923&view=rev Log: - Fixed bugs in the Share Printers dialog: Apply settings when the user only changed "deny remote access" to "allow remote access" if there is at least one real allow value. Having only "Listen *:631" in cupsd.conf is sufficient that the initSharing function sets listen_local to true. The modify_cupsd_conf tool works with 'Allow 127.0.0.2' and it logs its changes as comments in cupsd.conf. (See Novell/Suse Bugzilla bnc#441370) - Fixed dialog layout of the Share Printers dialog and of the Printing via Network dialog that both dialogs work also o.k. in ncurses mode. - 2.17.32 Modified: trunk/printer/VERSION trunk/printer/package/yast2-printer.changes trunk/printer/src/overview.ycp trunk/printer/src/printingvianetwork.ycp trunk/printer/src/sharing.ycp trunk/printer/tools/modify_cupsd_conf Modified: trunk/printer/VERSION URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/VERSION?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/VERSION (original) +++ trunk/printer/VERSION Wed Nov 5 14:00:50 2008 @@ -1 +1 @@ -2.17.31 +2.17.32 Modified: trunk/printer/package/yast2-printer.changes URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/package/yast2-printer.changes?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/package/yast2-printer.changes (original) +++ trunk/printer/package/yast2-printer.changes Wed Nov 5 14:00:50 2008 @@ -1,4 +1,21 @@ ------------------------------------------------------------------- +Wed Nov 5 11:30:40 CET 2008 - jsmeix@suse.de + +- Fixed bugs in the Share Printers dialog: + Apply settings when the user only changed + "deny remote access" to "allow remote access" + if there is at least one real allow value. + Having only "Listen *:631" in cupsd.conf is sufficient + that the initSharing function sets listen_local to true. + The modify_cupsd_conf tool works with 'Allow 127.0.0.2' + and it logs its changes as comments in cupsd.conf. + (See Novell/Suse Bugzilla bnc#441370) +- Fixed dialog layout of the Share Printers dialog + and of the Printing via Network dialog + that both dialogs work also o.k. in ncurses mode. +- 2.17.32 + +------------------------------------------------------------------- Tue Nov 4 13:30:59 CET 2008 - jsmeix@suse.de - Dialog layout fixes for the Share Printers dialog Modified: trunk/printer/src/overview.ycp URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/src/overview.ycp?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/src/overview.ycp (original) +++ trunk/printer/src/overview.ycp Wed Nov 5 14:00:50 2008 @@ -200,13 +200,13 @@ // on the official IPP port, a single error popup when the Overview dialog // is launched is sufficient. // The YaST printer modue does not support when the cupsd listens - // on a non-official port e.g. via "Listen *:9999" and/or "Port 9999" - // and/or "BrowsePort 9999" settings in /etc/cups/cupsd.conf. + // on a non-official port e.g. via "Listen *:1234" and/or "Port 1234" + // and/or "BrowsePort 1234" settings in /etc/cups/cupsd.conf. // Skip this test (and hope for the best) when netcat is not available - // because in yast2-printer.spec there can be only "Recommends: netcat" - // because there have been narrow-minded user complaints who insist - // not to have netcat installed because "netcat is a hacker intrusion tool" - // (explanations of the difference between netcat and e.g. nmap did not help). + // because in yast2-printer.spec there can be only "Recommends: netcat". + // There have been user complaints who insist not to have netcat installed + // because they insist that "netcat is a hacker intrusion tool" and thorough + // explanations of the difference between netcat and e.g. nmap did not help. if( Printerlib::ExecuteBashCommand( "ps -C cupsd" ) && Printerlib::ExecuteBashCommand( "type -P netcat" ) ) Modified: trunk/printer/src/printingvianetwork.ycp URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/src/printingvianetwork.ycp?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/src/printingvianetwork.ycp (original) +++ trunk/printer/src/printingvianetwork.ycp Wed Nov 5 14:00:50 2008 @@ -84,7 +84,7 @@ ) ), `HBox - ( `HSpacing( 3 ), + ( `HSpacing( 4 ), `VBox ( `Left ( `Label @@ -114,16 +114,7 @@ // A header for a TextEntry where the user can optionally // enter specific IP addresses and/or network/netmask // from where remote printer information is accepted: - _("Optional Specific IP Addresses or &Network/Netmask") - ) - ), - `Left - ( `Label - ( `id(`browse_allow_input_label), - // Label to show the syntax for the TextEntry where the user can - // enter specific IP addresses and/or network/netmask - // from where remote printer information is accepted: - _("(each nnn.nnn.nnn.nnn or nnn.nnn.nnn.nnn/mmm.mmm.mmm.mmm separated by one space)") + _("Optional Specific IP Addresses or &Network/Netmask (separated by space)") ) ), `Left @@ -158,7 +149,6 @@ ) ) ), - `VSpacing( 1 ), `Left ( `RadioButton ( `id(`client_only_radio_button), @@ -169,7 +159,7 @@ ) ), `HBox - ( `HSpacing( 3 ), + ( `HSpacing( 4 ), `HWeight ( 2, `TextEntry @@ -796,7 +786,6 @@ browse_allow_input_value = browse_allow_input_value + " "; initial_browse_allow_input_value = browse_allow_input_value; UI::ChangeWidget( `id(`browse_allow_input), `Value, browse_allow_input_value ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, false ); UI::ChangeWidget( `id(`client_only_radio_button), `Value, false ); UI::ChangeWidget( `id(`client_conf_server_name_input), `Enabled, false ); initial_server_name_input_value = Printerlib::client_conf_server_name; @@ -829,7 +818,6 @@ // it is useless to additionally accept it from specific IPs or networks: if( ! contains( Printerlib::cupsd_conf_browse_allow, "all" ) ) { UI::ChangeWidget( `id(`browse_allow_input), `Enabled, true ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, true ); } } else @@ -941,7 +929,6 @@ { UI::ChangeWidget( `id(`browse_allow_label), `Enabled, false ); UI::ChangeWidget( `id(`browse_allow_combo_box), `Enabled, false ); UI::ChangeWidget( `id(`browse_allow_input), `Enabled, false ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, false ); UI::ChangeWidget( `id(`client_conf_server_name_input), `Enabled, false ); UI::ChangeWidget( `id(`test_client_conf_server), `Enabled, false ); UI::ChangeWidget( `id(`connection_wizard), `Enabled, true ); @@ -950,7 +937,6 @@ { UI::ChangeWidget( `id(`browse_allow_label), `Enabled, true ); UI::ChangeWidget( `id(`browse_allow_combo_box), `Enabled, true ); UI::ChangeWidget( `id(`browse_allow_input), `Enabled, true ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, true ); UI::ChangeWidget( `id(`client_conf_server_name_input), `Enabled, false ); UI::ChangeWidget( `id(`test_client_conf_server), `Enabled, false ); UI::ChangeWidget( `id(`connection_wizard), `Enabled, true ); @@ -960,18 +946,15 @@ { // If browsing info is accepted from all hosts, // it is useless to additionally accept it from specific IPs or networks: UI::ChangeWidget( `id(`browse_allow_input), `Enabled, false ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, false ); } else { UI::ChangeWidget( `id(`browse_allow_input), `Enabled, true ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, true ); } } if( `client_only_radio_button == event["ID"]:nil ) { UI::ChangeWidget( `id(`browse_allow_label), `Enabled, false ); UI::ChangeWidget( `id(`browse_allow_combo_box), `Enabled, false ); UI::ChangeWidget( `id(`browse_allow_input), `Enabled, false ); - UI::ChangeWidget( `id(`browse_allow_input_label), `Enabled, false ); UI::ChangeWidget( `id(`client_conf_server_name_input), `Enabled, true ); UI::ChangeWidget( `id(`test_client_conf_server), `Enabled, true ); UI::ChangeWidget( `id(`connection_wizard), `Enabled, false ); Modified: trunk/printer/src/sharing.ycp URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/src/sharing.ycp?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/src/sharing.ycp (original) +++ trunk/printer/src/sharing.ycp Wed Nov 5 14:00:50 2008 @@ -24,7 +24,7 @@ * Summary: Print queue sharing and publishing dialog definition * Authors: Johannes Meixner * - * $Id: dialogs.ycp 27914 2006-02-13 14:32:08Z locilka $ + * $Id: sharing.ycp 27914 2006-02-13 14:32:08Z locilka $ */ { @@ -77,7 +77,7 @@ ) ), `HBox - ( `HSpacing( 3 ), + ( `HSpacing( 4 ), `VBox ( `Left ( `CheckBox @@ -106,24 +106,29 @@ ( `id(`interface_table_label), // A caption for a table to allow remote access to local print queues // via network interfaces specified in the table below: - _("Via network interfaces specified below") + _("Optionally via network interfaces specified below") ) ), - `Table - ( `id(`interface_table), - `opt(`keepSorting), - `header - ( // A table column header where the column lists network interfaces: - _("Interface"), - // A table column header where the column shows whether or not - // local print queues are published by default - // via the network interface in the other table column: - _("Publish printers by default via this interface") + `HBox + ( `VSquash + ( `MinHeight + ( 5, + `Table + ( `id(`interface_table), + `opt(`keepSorting), + `header + ( // A table column header where the column lists network interfaces: + _("Interface"), + // A table column header where the column shows whether or not + // local print queues are published by default + // via the network interface in the other table column: + _("Publish printers by default via this interface") + ), + initial_interface_table_items + ) + ) ), - initial_interface_table_items - ), - `Left - ( `HBox + `VBox ( `PushButton ( `id(`add_interface), // A PushButton label to add a network interface to the table which shows @@ -389,6 +394,16 @@ } // Any kind of allow_remote_access: // Check if there are real changes: + if( allow_remote_access && initial_deny_remote_access ) + { // When it is initially "deny remote access" + // and the user changed it to "allow remote access" + // but nothing else changed, then sharing_has_changed is true + // if there is at least one real allow value set. + // The last condition is true here because when allow_values is empty, + // it is a deny_remote_access case, see above. + sharing_has_changed = true; + } + // Check if there are real changes regarding the "@LOCAL" settings: if( allow_local_network_access != initial_allow_local_network_access || publish_to_local_network != initial_publish_to_local_network ) @@ -433,6 +448,8 @@ if( mergestring( current_browse_address_input_set, "" ) != mergestring( initial_browse_address_input_set, "" ) ) { sharing_has_changed = true; } + // Exit if no real change was detected above to avoid useless changes of cupsd.conf + // and subsequent useless restarts of the cupsd: if( ! sharing_has_changed ) { // Nothing has changed: return true; @@ -604,7 +621,12 @@ foreach( string listen_value, listen_values, { if( "" != listen_value ) - { if( "localhost" == listen_value ) + { if( "all" == listen_value ) + { listen_local = true; + listen_remote = true; + break; + } + if( "localhost" == listen_value ) { listen_local = true; } else @@ -614,7 +636,12 @@ } ); if( ! listen_local ) - { // 'Listen localhost' is mandatory (i.e. it is a broken config when it is missing). + { // A local running cupsd must mandatoty listen on the localhost interface + // (e.g. listen only on /var/run/cups/cups.sock is a broken config) + // but this does not mean that there must be a line "Listen localhost:631" + // in cupsd.conf because listening on all interfaces via "Listen *:631" + // lets it also listen on the localhost interface + // (see above how listen_local is set to true). // Try to do a simple fix for the broken config but ignore possible failures. // Set only 'Listen localhost:631' in /etc/cups/cupsd.conf which means // that all possibly existing non-'localhost' Listen entries are removed. Modified: trunk/printer/tools/modify_cupsd_conf URL: http://svn.opensuse.org/viewcvs/yast/trunk/printer/tools/modify_cupsd_conf?rev=52923&r1=52922&r2=52923&view=diff ============================================================================== --- trunk/printer/tools/modify_cupsd_conf (original) +++ trunk/printer/tools/modify_cupsd_conf Wed Nov 5 14:00:50 2008 @@ -73,6 +73,8 @@ exit 3 fi fi + # Log the current change: + echo "# $(date '+%F,%T') $MY_NAME $KEY $VALUE" >>$CUPSDCONF fi # Function to deal with 'Browsing [ On | Off ]' @@ -83,10 +85,10 @@ sed -i -e "s/^[[:space:]]*$KEY.*/$KEY $VALUE/i" $CUPSDCONF # There may exist no Browsing line: if ! grep -q "^$KEY $VALUE$" $CUPSDCONF - then # Append a Browsing line below the last 'Brows' line (ignore case) + then # Append a Browsing line below the last active 'Brows' line (ignore case) # to have the new Browsing line at the matching place # but if no 'Brows' line exists, append at the end of the file: - LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/Brows/I=' $CUPSDCONF | tail -n 1 )" + LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/^[^#]*Brows/I=' $CUPSDCONF | tail -n 1 )" if test -n "$LAST_BROWSE_LINE_NUMBER" then sed -i -e "${LAST_BROWSE_LINE_NUMBER}a$KEY $VALUE" $CUPSDCONF else echo -en "\n$KEY $VALUE\n\n" >>$CUPSDCONF @@ -119,10 +121,10 @@ # and finally deny from certain sources via BrowseDeny entries. # All existing active BrowseOrder lines are removed (ignore case). sed -i -e '/^[[:space:]]*BrowseOrder.*/Id' $CUPSDCONF - # Append the new BrowseOrder line below the last 'Brows' line (ignore case) + # Append the new BrowseOrder line below the last active 'Brows' line (ignore case) # to have the new BrowseOrder line at the matching place # but if no 'Brows' line exists, append at the end of the file: - LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/Brows/I=' $CUPSDCONF | tail -n 1 )" + LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/^[^#]*Brows/I=' $CUPSDCONF | tail -n 1 )" if test -n "$LAST_BROWSE_LINE_NUMBER" then sed -i -e "${LAST_BROWSE_LINE_NUMBER}aBrowseOrder allow,deny" $CUPSDCONF else echo -en "\nBrowseOrder allow,deny\n\n" >>$CUPSDCONF @@ -143,7 +145,7 @@ # and in the ordering of the values (needed for the test below): if ! test "none" = "$VALUE" then for V in $VALUE - do sed -i -e "/^BrowseOrder /i$KEY $V" $CUPSDCONF + do test -n "$V" && sed -i -e "/^BrowseOrder /i$KEY $V" $CUPSDCONF done else # The 'BrowseOrder allow,deny' line denies browse packets by default # to be 100% on the safe side have explicite 'BrowseAllow none' and 'BrowseDeny all' lines too: @@ -177,19 +179,18 @@ # Those lines are not removed because YaST does not support Port lines # so that a remove would silently delete Port information: sed -i -e 's/^[[:space:]]*\(Port.*\)$/#\1/i' $CUPSDCONF - # Determine the first 'Listen' line (ignore case): - FIRST_LISTEN_LINE_NUMBER="$( sed -n -e '/Listen/I=' $CUPSDCONF | head -n 1 )" + # Determine the first active 'Listen' line (ignore case): + FIRST_LISTEN_LINE_NUMBER="$( sed -n -e '/^[[:space:]]*Listen/I=' $CUPSDCONF | head -n 1 )" # All existing active Listen lines are removed (ignore case). # Those lines can be removed because YaST supports Listen lines # (YaST shows the Listen entries in the "share printers" dialog): sed -i -e '/^[[:space:]]*Listen.*/Id' $CUPSDCONF - # Append the mandatory 'Listen localhost' line (see http://www.cups.org/str.php?L2834 + # Insert the mandatory 'Listen localhost' line (see http://www.cups.org/str.php?L2834 # "What is *not* supported is a configuration where only a domain socket is enabled") - # below the first 'Listen' line (ignore case) - # to have the new Listen line at the matching place + # to have the new Listen line where the first active Listen line was # but if no 'Listen' line exists, append at the end of the file: if test -n "$FIRST_LISTEN_LINE_NUMBER" - then sed -i -e "${FIRST_LISTEN_LINE_NUMBER}aListen localhost:631" $CUPSDCONF + then sed -i -e "${FIRST_LISTEN_LINE_NUMBER}iListen localhost:631" $CUPSDCONF else echo -en "\nListen localhost:631\n\n" >>$CUPSDCONF fi # Append a Listen line for the domain socket (if such a line was there): @@ -204,7 +205,7 @@ then V="*" fi if ! test "localhost" = "$V" - then sed -i -e "/^Listen localhost/i$KEY $V:631" $CUPSDCONF + then test -n "$V" && sed -i -e "/^Listen localhost/i$KEY $V:631" $CUPSDCONF fi done fi @@ -215,7 +216,9 @@ # Remove trailing spaces (needed for the test below). # Remove '/var/run/cups/cups.sock' because it is only # an optional default (i.e. not really of interest). - RESULT="$( grep -i "^[[:space:]]*$KEY[[:space:]]" $CUPSDCONF | sed -e 's/[^ ]*\/cups\.sock//' -e 's/:631//' -e 's/\*/all/' | tr -s '[:blank:]' ' ' | sed -e 's/^ *//' | cut -s -d ' ' -f2 | tr -s '\n' ' ' | sed -e 's/ *$//' )" + # Unify a localhost (ignore case) or 127.0.0.1 value to "localhost". + # Replace the actual value '*' by the more meaningful word "all". + RESULT="$( grep -i "^[[:space:]]*$KEY[[:space:]]" $CUPSDCONF | sed -e 's/localhost/localhost/i' -e 's/127\.0*0\.0*0\.0*1/localhost/' -e 's/[^ ]*\/cups\.sock//' -e 's/:631//' -e 's/\*/all/' | tr -s '[:blank:]' ' ' | sed -e 's/^ *//' | cut -s -d ' ' -f2 | tr -s '\n' ' ' | sed -e 's/ *$//' )" echo -n "$RESULT" # For a nicer output on a terminal where stdout and stderr is mixed up, # output a '\n' on stderr to get subsequent stuff (e.g. the shell prompt @@ -237,7 +240,10 @@ # Function to deal with 'Allow [ all | @LOCAL | @IF(name) | host-address | network-address/netmask ] | Allow none' Allow() { if test -n "$VALUE" - then # Determine where the (first) root location starts (ignore case and ignore spaces): + then # Remove '127.0.0.2' from the value because it is set by default in any case + # and it is therefore also removed from the output of the resulting setting. + VALUE="$( echo $VALUE | sed -e 's/127\.00*\.00*\.0*2//g' | tr -s '[:space:]' ' ' | sed -e 's/^ *//' -e 's/ *$//' )" + # Determine where the (first) root location starts (ignore case and ignore spaces): ROOT_LOCATION_LINE_NUMBER="$( sed -n -e '/[[:space:]]*<[[:space:]]*Location[[:space:]]*\/[[:space:]]*>/I=' $CUPSDCONF | head -n 1 )" # Remove all root locations (ignore case and ignore spaces). # (If there is more than one root location it is a broken config file). @@ -261,7 +267,7 @@ # in the original ordering of the values (needed for the test below): if ! test "none" = "$VALUE" then for V in $( echo "$VALUE" | tac -s ' ' ) - do sed -i -e "${ROOT_LOCATION_LINE_NUMBER}a$KEY $V" $CUPSDCONF + do test -n "$V" && sed -i -e "${ROOT_LOCATION_LINE_NUMBER}a$KEY $V" $CUPSDCONF done else # The 'Order allow,deny' line denies packets by default # to be 100% on the safe side have explicite 'Allow none' and 'Deny all' lines too: @@ -294,16 +300,16 @@ # (YaST shows BrowseAddress entries in the "share printers" dialog): sed -i -e '/^[[:space:]]*BrowseAddress.*/Id' $CUPSDCONF if ! test "none" = "$VALUE" - then # Append the new BrowseAddress lines below the last 'Brows' line (ignore case) + then # Append the new BrowseAddress lines below the last active 'Brows' line (ignore case) # to have the new BrowseAddress lines at the matching place # but if no 'Brows' line exists, append at the end of the file. # Invert the ordering of the values to get the lines # in the original ordering of the values (needed for the test below): - LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/Brows/I=' $CUPSDCONF | tail -n 1 )" + LAST_BROWSE_LINE_NUMBER="$( sed -n -e '/^[^#]*Brows/I=' $CUPSDCONF | tail -n 1 )" for V in $( echo "$VALUE" | tac -s ' ' ) do if test -n "$LAST_BROWSE_LINE_NUMBER" - then sed -i -e "${LAST_BROWSE_LINE_NUMBER}a$KEY $V" $CUPSDCONF - else echo -en "\n$KEY $V\n" >>$CUPSDCONF + then test -n "$V" && sed -i -e "${LAST_BROWSE_LINE_NUMBER}a$KEY $V" $CUPSDCONF + else test -n "$V" && echo -en "\n$KEY $V\n" >>$CUPSDCONF fi done fi @@ -352,11 +358,11 @@ sed -i -e "s/^[[:space:]]*$KEY.*/$KEY $VALUE/i" $CUPSDCONF # There may exist no DefaultPolicy line: if ! grep -q "^$KEY $VALUE$" $CUPSDCONF - then # Append a DefaultPolicy line below the last 'Policy' line - # which is usually a '</Policy>' a 'ErrorPolicy' line (ignore case) + then # Append a DefaultPolicy line below the last active 'Policy' line + # which is usually a '</Policy>' or a 'ErrorPolicy' line (ignore case) # to have the new DefaultPolicy line at the matching place # but if no 'Policy' line exists, append at the end of the file: - LAST_POLICY_LINE_NUMBER="$( sed -n -e '/Policy/I=' $CUPSDCONF | tail -n 1 )" + LAST_POLICY_LINE_NUMBER="$( sed -n -e '/^[^#]*Policy/I=' $CUPSDCONF | tail -n 1 )" if test -n "$LAST_POLICY_LINE_NUMBER" then sed -i -e "${LAST_POLICY_LINE_NUMBER}a$KEY $VALUE" $CUPSDCONF else echo -en "\n$KEY $VALUE\n\n" >>$CUPSDCONF @@ -389,11 +395,11 @@ sed -i -e "s/^[[:space:]]*$KEY.*/$KEY $VALUE/i" $CUPSDCONF # There may exist no ErrorPolicy line: if ! grep -q "^$KEY $VALUE$" $CUPSDCONF - then # Append a ErrorPolicy line below the last 'Policy' line + then # Append a ErrorPolicy line below the last active 'Policy' line # which is usually a '</Policy>' or a 'DefaultPolicy' line (ignore case) # to have the new ErrorPolicy line at the matching place # but if no 'Policy' line exists, append at the end of the file: - LAST_POLICY_LINE_NUMBER="$( sed -n -e '/Policy/I=' $CUPSDCONF | tail -n 1 )" + LAST_POLICY_LINE_NUMBER="$( sed -n -e '/^[^#]*Policy/I=' $CUPSDCONF | tail -n 1 )" if test -n "$LAST_POLICY_LINE_NUMBER" then sed -i -e "${LAST_POLICY_LINE_NUMBER}a$KEY $VALUE" $CUPSDCONF else echo -en "\n$KEY $VALUE\n\n" >>$CUPSDCONF -- To unsubscribe, e-mail: yast-commit+unsubscribe@opensuse.org For additional commands, e-mail: yast-commit+help@opensuse.org