Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) #234

doktornotor · 2016-12-17T13:14:42Z

Make it possible to disable the package without uninstalling (e.g. to prevent timeout delays when there's no WAN connection). Also, it is now possible to delete the stored credentials from config.xml.

Some tweaks to avoid completely bogus "success" message for Bug #6950 while here. Now it should at least report failure when the write_config() failed due to the user having user-config-readonly privilege, instead of always claiming it worked.

Remove useless stuff. There is nothing to check here for success, except possibly write_config() result, but that's not the point of this feature at all.

We need some feedback for the users that the thing did something - it takes considerable time to finish and not providing any result is just badly confusing. Instead of always claiming success, at least check whether write_config() worked.

jim-p

Looks good, could use a couple of very minor improvements but reads through OK.

jim-p · 2016-12-19T15:18:58Z

sysutils/pfSense-pkg-AutoConfigBackup/files/usr/local/pkg/autoconfigbackup.inc

 	if (function_exists("platform_booting")) {
 		if (platform_booting()) {
 			return;
 		}
 	} elseif ($g['booting']) {
 		return;
+	} elseif (!acb_enabled()) {


This could be combined with the conditional above it.

jim-p · 2016-12-19T15:19:03Z

sysutils/pfSense-pkg-AutoConfigBackup/files/usr/local/pkg/autoconfigbackup.inc

 	if (function_exists("platform_booting")) {
 		if (platform_booting()) {
 			return;
 		}
 	} elseif ($g['booting']) {
 		return;
+	} elseif (!acb_enabled()) {


This could be combined with the conditional above it.

@jim-p - Before redoing it. Is the first check about function_exists() still needed here? Was a 2.2.x compatibility thing IIRC, or even earlier?

That can go. That function was added on 2.2, and this code wouldn't ever touch anything that old so it can be cleaned up.

jim-p

Looks better. Applied it as a patch and noticed one sizable flaw. By default it is not enabled. So if someone upgrades, they would be surprised to find that ACB isn't working. The acb_enabled() test shouldn't just check for 'on' but also consider the variable being unset as on. If it's set but empty then it should be considered disabled.

doktornotor · 2016-12-19T16:54:20Z

Hmmm, well... Given my experience with completely retared isset() behaviour in PHP, I'd frankly leave the check well alone since there's nothing wrong with that and it completely avoids stupid issues such as this (as the very recent example): #232.

Perhaps someone has a way to upgrade people to default it to on if defaulting it to on in the XML does not do that. PHP's isset() is broken by design.

jim-p · 2016-12-19T17:00:46Z

Maybe instead of a checkbox, use a drop-down box with Enabled/Disabled and then the value could explicitly default to enabled and the disabled value would be spelled out as well. Then unset/empty could be considered equal to enabled.

jim-p · 2016-12-19T17:59:01Z

That works great! It assumed enabled and acted as expected when disabled/manually enabled. It's also more obvious to the user this way.

doktornotor · 2016-12-19T18:00:24Z

@jim-p - thanks for testing/help.

Is there a ticket for:

setting up defaults on package install
setting up defaults on package upgrade
renaming vars on upgrade?

Seriously needed. This hacking around sucks.

jim-p · 2016-12-19T18:02:23Z

None that I'm aware of. We don't have any kind of special code path to handle upgrades for packages that I know of, they've been left to fend for themselves individually or maintain backward compatibility. It could use improvement, surely, but it might take some more in-depth design thought on how best to handle it all.

doktornotor · 2016-12-19T18:06:09Z

Indeed, still I'd say it'd be worth it, looking at the amount of cruft that accumulated in packages like Squid is kinda sad.

- update to 1.4.10 Broker Fix TLS operation with websockets listeners and libwebsockets 2.x. Closes pfsense#186. Don.t disconnect client on HUP before reading the pending data. Closes pfsense#7. Fix some $SYS messages being incorrectly persisted. Closes pfsense#191. Support OpenSSL 1.1.0. Call fsync after persisting data to ensure it is correctly written. Closes pfsense#189. Fix persistence saving of subscription QoS on big-endian machines. Fix will retained flag handling on Windows. Closes pfsense#222. Broker now displays an error if it is unable to open the log file. Closes pfsense#234. Client library Support OpenSSL 1.1.0. Fixed the C++ library not allowing SOCKS support to be used. Closes pfsense#198. Fix memory leak when verifying a server certificate with a subjectAltName section. Closes pfsense#237. Build Don.t attempt to install docs when WITH_DOCS=no. Closes pfsense#184. PR: 213532 Submitted by: ohauer Approved by: maintainer <joe@thrallingpenguin.com> Approved by: ports-secteam (feld)

[ Robert Edmonds ] * Release 1.3.0. * Add test case for the issue in #220 (#254). * Fix issue #251, "Bad enums with multiple oneofs" (#256). * Add warning flags to my_CFLAGS (#257). * Fix namespace errors when compiled with latest protobuf (#280). * Bump minimum required header version for proto3 syntax (#282). [ Paolo Borelli ] * Turn the compiler into a protoc plugin (#206). This allows the protobuf-c compiler to be invoked as "protoc --c_out=...". For backwards compatibility, we still ship a protoc-c command, but it's a symlink to the protoc-gen-c binary. * proto3 support (#228). * Remove leftover FIXME comment (#258). * Fix proto3 "is zeroish" evaluation (#264). * Small cleanup in oneof handling (#265). * Rework is_zeroish one more time (#267). * proto3: make strings default to "" instead of NULL (#274). [ Tomek Wasilczyk ] * Fix -Wsign-compare warnings (#213). * Fix ISO C90 -Wdeclaration-after-statement warnings (#214). * Fix bigendian -Wunused-label warning (#215). [ Ilya Lipnitsky ] * protoc-c/c_message.cc: Force int size on oneof enums (#221). Fixes wrong enum generation and handling for onceof cases (#220). [ Adnan ] * Fix cmake build if built as part of an external project (#231). [ Gregory Detal ] * Remove .pb.{cc,h} in distdir instead of top_distdir in order to prevent removing files from other projects when protobuf-c is included as an autotools subproject (#232). [ Ben Farnham ] * Relax autoconf constraint from v2.64 to v2.63 so that it works on older Linux distros (#233). [ Thomas Koeckerbauer ] * rm argument fix for Solaris (#234). * Add 'const' qualifier to 'init_value' variable in generated files (#236). [ Richard Kettlewell ] * Document and extend the effect of passing NULL to ..._free_unpacked functions (#255). [ Alex Milich ] * CMake: Workaround for static builds that use MSVC (#243). [ Josh Junon ] * CMake: Allow protobuf-c to be included via include_subdirectory (#245). [ Alexei Kasatkin ] * CMake: Windows fixes (#266). This fixes the build breakage with devel/protobuf 1.3.0 [1] PR: 221572 [1] Sponsored by: Farsight Security, Inc.

Major changes between sudo 1.9.13 and 1.9.12p2: * Fixed a bug running relative commands via sudo when "log_subcmds" is enabled. GitHub issue #194. * Fixed a signal handling bug when running sudo commands in a shell script. Signals were not being forwarded to the command when the sudo process was not run in its own process group. * Fixed a bug in cvtsudoers' LDIF parsing when the file ends without a newline and a backslash is the last character of the file. * Fixed a potential use-after-free bug with cvtsudoers filtering. GitHub issue #198. * Added a reminder to the default lecture that the password will not echo. This line is only displayed when the pwfeedback option is disabled. GitHub issue #195. * Fixed potential memory leaks in error paths. GitHub issues #199, #202. * Fixed potential NULL dereferences on memory allocation failure. GitHub issues #204, #211. * Sudo now uses C23-style attributes in function prototypes instead of gcc-style attributes if supported. * Added a new "list" pseudo-command in sudoers to allow a user to list another user's privileges. Previously, only root or a user with the ability to run any command as either root or the target user on the current host could use the -U option. This also includes a fix to the log entry when a user lacks permission to run "sudo -U otheruser -l command". Previously, the logs would indicate that the user tried to run the actual command, now the log entry includes the list operation. * JSON logging now escapes control characters if they happen to appear in the command or environment. * New Albanian translation from translationproject.org. * Regular expressions in sudoers or logsrvd.conf may no longer contain consecutive repetition operators. This is implementation- specific behavior according to POSIX, but some implementations will allocate excessive amounts of memory. This mainly affects the fuzzers. * Sudo now builds AIX-style shared libraries and dynamic shared objects by default instead of svr4-style. This means that the default sudo plugins are now .a (archive) files that contain a .so shared object file instead of bare .so files. This was done to improve compatibility with the AIX Freeware ecosystem, specifically, the AIX Freeware build of OpenSSL. Sudo will still load svr4-style .so plugins and if a .so file is requested, either via sudo.conf or the sudoers file, and only the .a file is present, sudo will convert the path from plugin.so to plugin.a(plugin.so) when loading it. This ensures compatibility with existing configurations. To restore the old, pre-1.9.13 behavior, run configure using the --with-aix-soname=svr4 option. * Sudo no longer checks the ownership and mode of the plugins that it loads. Plugins are configured via either the sudo.conf or sudoers file which are trusted configuration files. These checks suffered from time-of-check vs. time-of-use race conditions and complicate loading plugins that are not simple paths. Ownership and mode checks are still performed when loading the sudo.conf and sudoers files, which do not suffer from race conditions. The sudo.conf "developer_mode" setting is no longer used. * Control characters in sudo log messages and "sudoreplay -l" output are now escaped in octal format. Space characters in the command path are also escaped. Command line arguments that contain spaces are surrounded by single quotes and any literal single quote or backslash characters are escaped with a backslash. This makes it possible to distinguish multiple command line arguments from a single argument that contains spaces. * Improved support for DragonFly BSD which uses a different struct procinfo than either FreeBSD or 4.4BSD. * Fixed a compilation error on Linux arm systems running older kernels that may not define EM_ARM in linux/elf-em.h. GitHub issue #232. * Fixed a compilation error when LDFLAGS contains -Wl,--no-undefined. Sudo will now link using -Wl,--no-undefined by default if possible. GitHub issue #234. * Fixed a bug executing a command with a very long argument vector when "log_subcmds" or "intercept" is enabled on a system where "intercept_type" is set to "trace". GitHub issue #194. * When sudo is configured to run a command in a pseudo-terminal but the standard input is not connected to a terminal, the command will now be run as a background process. This works around a problem running sudo commands in the background from a shell script where changing the terminal to raw mode could interfere with the interactive shell that ran the script. GitHub issue #237. * A missing include file in sudoers is no longer a fatal error unless the error_recovery plugin argument has been set to false. PR: 269563 Submitted by: cy Reported by: cy Approved by: garga MFH: 2023Q1

doktornotor added 3 commits December 17, 2016 14:05

Disable Auto Config Backup without uninstalling (Feature #6951)

0bdc6f0

Disable Auto Config Backup without uninstalling (Feature #6951)

2dccd9a

Bump port version

5da52be

netgate-git-updates added the CLA label Dec 17, 2016

Remove potentially misleading "success" message (Bug #6950)

82f98fb

Remove useless stuff. There is nothing to check here for success, except possibly write_config() result, but that's not the point of this feature at all.

doktornotor changed the title ~~Disable Auto Config Backup without uninstalling (Feature #6951)~~ Disable Auto Config Backup feature (Feature #6951), remove bogus success message (Bug #6950) Dec 17, 2016

doktornotor changed the title ~~Disable Auto Config Backup feature (Feature #6951), remove bogus success message (Bug #6950)~~ Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) Dec 17, 2016

rbgarga requested review from jim-p and rbgarga December 19, 2016 15:16

jim-p requested changes Dec 19, 2016

View reviewed changes

Remove unneeded pre-2.2 compatibility checks and simplify

2c0f450

jim-p requested changes Dec 19, 2016

View reviewed changes

doktornotor added 2 commits December 19, 2016 18:45

Change to select instead to have explicit values for enabled/disabled

fe5c2fb

Consider the package to be enabled If not explicitly disabled

fb00e44

jim-p approved these changes Dec 19, 2016

View reviewed changes

rbgarga approved these changes Dec 19, 2016

View reviewed changes

netgate-git-updates merged commit fb00e44 into pfsense:devel Dec 20, 2016

netgate-git-updates pushed a commit that referenced this pull request Dec 20, 2016

Merge pull request #234 from doktornotor/patch-3

e06176b

doktornotor deleted the patch-3 branch December 20, 2016 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) #234

Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) #234

doktornotor commented Dec 17, 2016 •

edited

jim-p left a comment

jim-p Dec 19, 2016

jim-p Dec 19, 2016

doktornotor Dec 19, 2016 •

edited

jim-p Dec 19, 2016

jim-p left a comment

doktornotor commented Dec 19, 2016 •

edited

jim-p commented Dec 19, 2016

jim-p commented Dec 19, 2016

doktornotor commented Dec 19, 2016

jim-p commented Dec 19, 2016

doktornotor commented Dec 19, 2016

Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) #234

Disable Auto Config Backup feature (Feature #6951), add some tweaks for savemsg (Bug #6950) #234

Conversation

doktornotor commented Dec 17, 2016 • edited

jim-p left a comment

Choose a reason for hiding this comment

jim-p Dec 19, 2016

Choose a reason for hiding this comment

jim-p Dec 19, 2016

Choose a reason for hiding this comment

doktornotor Dec 19, 2016 • edited

Choose a reason for hiding this comment

jim-p Dec 19, 2016

Choose a reason for hiding this comment

jim-p left a comment

Choose a reason for hiding this comment

doktornotor commented Dec 19, 2016 • edited

jim-p commented Dec 19, 2016

jim-p commented Dec 19, 2016

doktornotor commented Dec 19, 2016

jim-p commented Dec 19, 2016

doktornotor commented Dec 19, 2016

doktornotor commented Dec 17, 2016 •

edited

doktornotor Dec 19, 2016 •

edited

doktornotor commented Dec 19, 2016 •

edited