Saturday, January 30, 2010

DynDNS Update Client Shell Script

I'm sharing the shell script I wrote and have been using for the past number of months to update my Dynamic DNS account on DynDNS.com with my dynamic IP address on my DSL connection, and previously with Alltel / Verizon Wireless.

There are many other update clients available. However, I had specific issues with every other one I tried, and this one meets my needs perfectly. Additionally, I wrote this script with a few particular design goals, as commented in the code below.

This is a Unix/Linux shell script, and is not designed to work with other environments such as Windows. It is also written as a generic POSIX-compliant shell script, as I took extra caution to avoid "bashisms". It may work under Cygwin or another such environment as long as all the dependencies are available and any necessary changes are made. These dependencies include: wget, date, logger, sed, at, and parseable output from ifconfig - all available by default on most Linux installations.

#!/bin/sh

# Mark A. Ziesemer, http://www.ziesemer.com
# 2009-08-30, 2009-10-26
# http://blogger.ziesemer.com/2010/01/dyndns-update-client-shell-script.html
# With thanks to "ferret" and "pgas" on #bash (IRC) for some general bash-related questions.

# Design goals, in order of priority / importance:
#    1) 100% compliance with DynDNS Update API (http://www.dyndns.com/developers/specs/),
#      including all policies and required timings.
#    2) Maintain and update IP as fast as allowed by the specification, minimizing "downtime".
#     Properly update in cases currently missed by other update clients, e.g.
#        when multiple updates are requested within an otherwise arbitrarily-defined time limit.
#    3) Run efficiently, respecting CPU, RAM, and disk requirements.
#      Use external scheduling (atd) and hooks to lessen required in-memory process time as much as possible.
#      Should be re-usable on embedded systems, e.g. OpenWrt, with only minor modifications necessary.
#    4) No dependencies on other "large" runtime libraries, e.g. Perl or Python.

# Exit status codes:
#   0   Update completed successfully.
#   11  Update completed successfully, but unnecessarily. (NOCHG)
#   21  IP has not changed.
#   31  Recognized temporary failure.
#   41  Assumed (unrecognized) temporary failure.
#   51  Temporary failure due to $lastUpdateResult="TEMP_FAIL".
#   101  Permanent failure.
#   111  Permanent failure due to $lastUpdateResult="FATAL".
#   112  Permanent failure due to unrecognized $lastUpdateResult.
#   201  Couldn't kill existing script.
#   255  Other unexpected failure.

### variables accepted as command line arguments
configFile="/etc/ddIpUpdate/ddIpUpdate.config"
forceUpdate=
forceRetry=

### variables persisted in $configFile
dynIF=

username=
password=
hostname=

varDir="/var/lib/ddIpUpdate"
stateFile="${varDir}/ddIpUpdate.state"
pidFile="/var/run/ddIpUpdate.pid"

### variables persisted in $stateFile

lastIP=
lastUpdateTime=
# GOOD, TEMP_FAIL, FATAL
lastUpdateResult=
futureJobNum=

### other internal variables
userAgent="com.ziesemer.ddIpUpdate 2009.10.26"
callback=$0

### Helper functions.

_log(){
  printf "$(date --rfc-3339=seconds) $*\n" >&2
  logger $0 "$*"
}

_setArgs(){
  while [ "$1" != "" ]; do
    case $1 in
      "-c" | "--configFile")
        shift
        configFile=$1
        ;;
      "-f" | "--forceUpdate")
        forceUpdate=true
        ;;
      "-r" | "--forceRetry")
        forceRetry=true
        ;;
    esac
    shift
  done
}

_exitErr(){
  local exitStatus=$?
  _log "Error! $exitStatus"
  _exitNormal
  return $exitStatus
}

_exitNormal(){
  trap - EXIT
  _writeConfig
  rm $pidFile
}

_checkRun(){ # cmd
  local cmd status out
  cmd=$*
  out=$(eval $cmd)
  status=$?
  if [ $status -ne 0 ]; then
    _log \
      "\nUnexpected return status: $status, exiting." \
      "\nCommand: $cmd" \
      "\nOutput: $out"
    return $status
  else
    echo "$out"
  fi
}

_schedule(){ # cmd, time
  if [ -n "$futureJobNum" ]; then
    _log "Removing existing scheduled job: $futureJobNum"
    atrm $futureJobNum
  fi

  local at_out
  at_out=$(_checkRun "echo $1 | at $2 2>&1")
  _log "Scheduled command: \"$1\", $(echo "$at_out" | tail -n 1)"
  local at_id=$(echo "$at_out" | sed -n "s/^job \([0-9]*\) at .*$/\1/p")
  echo $at_id
}

### Core functions.

_findIP(){
  local ip=$(ifconfig $dynIF | sed -n 's/ *inet addr:\([0-9.]*\).*/\1/p')
  _log "Detected IP $ip on interface $dynIF."
  echo $ip
}

_checkInstances(){
  if [ -e $pidFile ]; then
    local pid=$(cat $pidFile)
    command kill -TERM $pid
    # 'wait' only works for child processes
    sleep 1
    if [ kill -0 "$pid" ]; then
      _log "Existing script with PID $pid didn't stop; exiting..."
      exit 201
    fi
  fi

  echo $$ > $pidFile
}

_checkLastStatus(){
  case "$lastUpdateResult" in
    "FATAL")
      _log "Last update resulted in a fatal condition; user intervention required."
      exit 111
      ;;
    "TEMP_FAIL")
      if [ -z "$forceRetry" ] ; then
        if [ $(( $(date +%s) < ($lastUpdateTime + 1800) )) -ne 0 ] ; then
          _log "Temporary timeout not yet expired, or user intervention required."
          _rescheduleTempFail
          exit 51
        fi
      fi
      ;;
    "GOOD" | "")
      # Continue
      ;;
    *)
      _log "Unrecognized lastUpdateResult: $lastUpdateResult; exiting..."
      exit 112
      ;;
  esac
}

_readConfig(){
  case "$configFile" in
    *"/"*) . $configFile ;;
    *) . ./$configFile ;;
  esac
  if [ -r $stateFile ]; then
    case "$stateFile" in
      *"/"*) . $stateFile ;;
      *) . ./$stateFile ;;
    esac
  fi
}

_writeConfig(){
  echo "#This file is automatically re-written!" >$stateFile
  for name in "lastIP" "lastUpdateTime" "lastUpdateResult" "futureJobNum"; do
    echo $name=\"$(eval "echo \$$name")\" >> $stateFile
  done
  echo >> $stateFile
}

_checkIPChanged(){ # ip
  if [ "$lastIP" = "$1" ]; then
    _log "IP has not changed from $lastIP; exiting..."
    exit 21
  fi
}

_rescheduleTempFail(){
  futureJobNum=$(_schedule "$0 -c $configFile" "now + 30 minutes")
}

_updateIP(){ # ip
  local returnStatus=255
  lastIP=

  # Could do without writing the temporary files, but good to save anyway for debugging / troubleshooting.
  local updateResult=0
  echo "https://$username:$password@members.dyndns.org/nic/update?hostname=$hostname&myip=$1" \
    | wget -i - -O - -U "$userAgent" --save-headers \
      2>${varDir}/response.err >${varDir}/response.out || updateResult=$?
  
  if [ $updateResult -eq 0 ]; then
    # DynDNS requires action based on return codes only, not HTTP status:
    #    http://www.dyndns.com/developers/specs/guidelines.html
    :
  else
    _log "Error result from web service: $updateResult"
  fi

  # Previous bashism (bash array):
  # declare -a updateResponse=($(tail -n 1 ${varDir}/response.out))
  # ${updateResponse[0]}

  local updateResponse="$(tail -n 1 ${varDir}/response.out)"
  local updateResponse1="$(echo $updateResponse | awk '{print $1}')"
  # 2nd token is the returned IP, which really doesn't offer anything.
  # local updateResponse2="$(echo $updateResponse | awk '{print $2}')"

  _log "Received response: $updateResponse"

  case "$updateResponse1" in
    "good")
      lastUpdateResult="GOOD"
      returnStatus=0
      ;;
    "nochg")
      lastUpdateResult="GOOD"
      returnStatus=11
      ;;
    "dnserr" | "911")
       # Temporary issue, prevent any further requests for at least 30 minutes or until user manually clears error.
      lastUpdateResult="TEMP_FAIL"
      returnStatus=31
      ;;
    "badauth" | "!donator" | "notfqdn" | "nohost" | "numhost" | "abuse" | "badagent" | *)
      if ( [ $updateResult -ne 0 ] && [ -z "$updateResponse1" ] ); then
        # Temporary network failure or other assumed-temporary issue.
        lastUpdateResult="TEMP_FAIL"
        returnStatus=41
      else
        # Known permanent failure, or other completely unexpected result.
        # Prevent any further requests until user manually clears error.
        lastUpdateResult="FATAL"
        returnStatus=101
      fi
      ;;
  esac

  lastUpdateTime=$(date +%s)
  case "$lastUpdateResult" in
    "GOOD")
      lastIP=$1
      futureJobNum=$(_schedule "$0 -f -c $configFile" "now + 28 days")
      ;;
    "TEMP_FAIL")
      _rescheduleTempFail
      ;;
  esac

  return $returnStatus
}

_runUpdate(){
  _checkLastStatus
  local ip=$(_findIP)
  if [ -z "$forceUpdate" ]; then
    _checkIPChanged $ip
  fi
  _updateIP $ip
  return $?
}

### "Main"

set -e
trap _waitAbort TERM
trap _exitErr EXIT

_setArgs $*
_readConfig
_checkInstances

mkdir -p $varDir

result=0
_runUpdate || result=$?

_exitNormal
_log "Exiting with status: $result"
exit $result

To use, just save as an executable file somewhere at the location of your choice. Create a configuration file containing the 4 required parameters: dynIF, username, password, and hostname, e.g.:

dynIF="ppp0"
username="someUsername"
password="somePassword"
hostname="someDomainName.dyndns.org"

View the source above for the other optional parameters accepted, as well as their default values. The permissions for this file should be set so that it is only readable by the user account that will execute the script, in order to protect the password. This file will be looked for at "/etc/ddIpUpdate/ddIpUpdate.config" by default, but can be overridden with the "--configFile" or "-c" command line arguments.

When run, this script maintains state in a file, the location of which defaults to "/var/lib/ddIpUpdate/ddIpUpdate.state", but can be overriden in the config file. This file is created automatically on first run, and looks like this:

#This file is automatically re-written!
lastIP="1.2.3.4"
lastUpdateTime="1264001234"
lastUpdateResult="GOOD"
futureJobNum="1"

There are many options available for having this script executed. On my Ubuntu Karmic (9.10) system, I created a link to this script in the "/etc/ppp/ip-up.d" directory so that it is executed every time after my PPP connection starts, or is restarted.

I don't write shell scripts for a living, so while I have been using this myself for several months now without an issue, it is certainly possible that there may be a bug or other room for improvement. Please leave a comment here if you have a correction or a suggestion, but please "cite your source" to a supporting reference related to the issue, particularly for any shell script semantics. Please also remember to follow typical best practices for bug reporting.

Thursday, January 28, 2010

Redundant argument validation code in Java IO classes

Today I noticed some almost comical redundant checks in a number of the java.io InputStream, OutputStream, Reader, and Writer classes. I'm looking at the read and write methods with a signature of ([] x, int off, int len), where "[] x" is either a byte or char array, and "x" is named "b" for a byte array, or "c" or "cbuf" for a char array. One example is InputStream.read(byte[], int, int).

Below is a portion of the source code used within StringReader, StringWriter, BufferedReader, BufferedWriter, CharArrayReader, CharArrayWriter, and ByteArrayOutputStream, identical between all versions of Java checked from 1.3 - 1.6 (6.0), and only slightly reformatted for readability:

if ( (off < 0) || (off > x.length) || (len < 0)
    || ((off + len) > x.length) || ((off + len) < 0) ) {
  throw new IndexOutOfBoundsException();
}

Keep in mind that these methods are typically called within a loop, and depending upon the chosen array/buffer size and the amount of data being read or written, these methods and the shown checks may be executed many times. Any unnecessary statements will only hinder performance. Notice any redundancies in the check?

First, if neither "off" nor "len" are less than 0 (both already checked), it is guaranteed that the sum of "off" and "len" will also never be less than 0. This makes the "((off + len) < 0)" check completely redundant, and it should be removed.

Second, if the sum of "off" and "len" is not greater than the length of the array, and if both "off" and "len" are positive (all already checked), it is guaranteed that "off" alone will also never be greater than the length of the array. This makes the "(off > cbuf.length)" check completely redundant, and it should be removed.

This would simplify and shorten the above checks to the following:

if ( (off < 0) || (len < 0) || ((off + len) > x.length) ) {
  throw new IndexOutOfBoundsException();
}

While I haven't checked, I suppose it is possible that this type of issue could be optimized away by the compiler or even the JVM at runtime - but I wouldn't hold my breath.

BufferedOutputStream in all the same versions works a little differently and doesn't perform any of its own argument validation.

Someone apparently realized this issue on ByteArrayInputStream, and made an optimization. In Java 1.5 / 5.0 and previous, the check was the same as all the others above. In Java 1.6 / 6.0, the check is now written as:

if ( off < 0 || len < 0 || len > x.length - off ) {
  throw new IndexOutOfBoundsException();
}

This is basically identical to my optimized version above, just with "off" moved to the other side of the comparison, with the operator properly switched from '+' to '-' to match. However, all versions of the read method in ByteArrayInputStream add an unnecessary check for "(b == null)", only to manually throw a NullPointerException. An identical NullPointerException will already be thrown on the attempt to read the "length" property from the array if the array is null in the above check. This makes the additional check redundant, and it should be removed.

BufferedInputStream takes an interesting, alternative approach:

if ( (off | len | (off + len) | (x.length - (off + len))) < 0) {
  throw new IndexOutOfBoundsException();
}

Notice that bitwise ORs are being used instead of logical ORs. Any negative value bitwise OR'd with any other value(s) will produce a negative result. However, this code is performing another redundant operation. Again, if neither "off" nor "len" are less than 0 (both already checked), it is guaranteed that the sum of "off" and "len" will also never be less than 0. This makes the "(off + len))" check completely redundant, and should be removed. This can simplify and shorten to:

if ( (off | len | (x.length - (off + len))) < 0 ) {
  throw new IndexOutOfBoundsException();
}

I'm not certain which of the approaches (bitwise or logical ORs) should be faster. Successive logical ORs can be skipped once an earlier portion evaluates to true. However, for almost all (assuming valid) calls to these methods, these expressions should almost always evaluate to false, which then requires an entire logical OR expression to be evaluated regardless. I'd be interested to hear any detailed arguments for one way or another.

I reported this to the Sun (Oracle) bug database (internal review ID 1705260), and will post an update if and when I receive a public bug ID.