Modify behaviour when receiving an invalid RTSP packet to be the same…

… as a genuine AirPort Express, but time out after two minutes anyway. Also move the retain_artwork flag to diagnostics.

rtsp.c

@@ -585,7 +585,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
  int release_buffer = 0; // on exit, don't deallocate the buffer if everything was okay
  char *buf = malloc(buflen + 1); // add a NUL at the end
  if (!buf) {
- warn("rtsp_read_request: can't get a buffer.");
+ warn("Connection %d: rtsp_read_request: can't get a buffer.", conn->connection_number);
  return (rtsp_read_request_response_error);
  }
  pthread_cleanup_push(malloc_cleanup, buf);
@@ -595,7 +595,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
 
  while (msg_size < 0) {
  if (conn->stop != 0) {
- debug(3, "RTSP conversation thread %d shutdown requested.", conn->connection_number);
+ debug(3, "Connection %d: shutdown requested.", conn->connection_number);
  reply = rtsp_read_request_response_immediate_shutdown_requested;
  goto shutdown;
  }
@@ -604,7 +604,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
 
  if (nread == 0) {
  // a blocking read that returns zero means eof -- implies connection closed
- debug(3, "RTSP conversation thread %d -- connection closed.", conn->connection_number);
+ debug(3, "Connection %d: -- connection closed.", conn->connection_number);
  reply = rtsp_read_request_response_channel_closed;
  goto shutdown;
  }
@@ -613,7 +613,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
  if (errno == EINTR)
  continue;
  if (errno == EAGAIN) {
- debug(1, "Getting Error 11 -- EAGAIN from a blocking read!");
+ debug(1, "Connection %d: getting Error 11 -- EAGAIN from a blocking read!", conn->connection_number);
  continue;
  }
  if (errno != ECONNRESET) {
@@ -643,8 +643,8 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
  msg_size = msg_handle_line(the_packet, buf);
 
  if (!(*the_packet)) {
- warn("Closing the session because an RTSP header was missing in an incoming packet.");
- reply = rtsp_read_request_response_read_error;
+ debug(1,"Connection %d: rtsp_read_request can't find an RTSP header.", conn->connection_number);
+ reply = rtsp_read_request_response_bad_packet;
  goto shutdown;
  }
 
@@ -657,7 +657,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
  if (msg_size > buflen) {
  buf = realloc(buf, msg_size + 1);
  if (!buf) {
- warn("too much content");
+ warn("Connection %d: too much content.", conn->connection_number);
  reply = rtsp_read_request_response_error;
  goto shutdown;
  }
@@ -2409,16 +2409,6 @@ static void *rtsp_conversation_thread_func(void *pconn) {
  debug(debug_level, "Connection %d: RTSP Response:", conn->connection_number);
  debug_print_msg_headers(debug_level, resp);
 
- /*
- fd_set writefds;
- FD_ZERO(&writefds);
- FD_SET(conn->fd, &writefds);
- do {
- memory_barrier();
- } while (conn->stop == 0 &&
- pselect(conn->fd + 1, NULL, &writefds, NULL, NULL, &pselect_sigset) <= 0);
- */
-
  if (conn->stop == 0) {
  int err = msg_write_response(conn->fd, resp);
  if (err) {
@@ -2470,6 +2460,17 @@ static void *rtsp_conversation_thread_func(void *pconn) {
  } else {
  tstop = 1;
  }
+ } else if (reply == rtsp_read_request_response_bad_packet) {
+ char *response_text = "RTSP/1.0 400 Bad Request\r\nServer: AirTunes/105.1\r\n\r\n";
+ ssize_t reply = write(conn->fd, response_text, strlen(response_text));
+ if (reply == -1) {
+ char errorstring[1024];
+ strerror_r(errno, (char *)errorstring, sizeof(errorstring));
+ debug(1, "rtsp_read_request_response_bad_packet write response error %d: \"%s\".", errno, (char *)errorstring);
+ } else if (reply != (ssize_t)strlen(response_text)) {
+ debug(1, "rtsp_read_request_response_bad_packet write %d bytes requested but %d written.", strlen(response_text),
+ reply);
+ }
  } else {
  debug(1, "Connection %d: rtsp_read_request error %d, packet ignored.",
  conn->connection_number, (int)reply);

scripts/shairport-sync.conf

@@ -192,7 +192,6 @@ metadata =
 // enabled = "yes"; // set this to yes to get Shairport Sync to solicit metadata from the source and to pass it on via a pipe
 // include_cover_art = "yes"; // set to "yes" to get Shairport Sync to solicit cover art from the source and pass it via the pipe. You must also set "enabled" to "yes".
 // cover_art_cache_directory = "/tmp/shairport-sync/.cache/coverart"; // artwork will be stored in this directory if the dbus or MPRIS interfaces are enabled or if the MQTT client is in use. Set it to "" to prevent caching, which may be useful on some systems
-// retain_cover_art = "no"; // artwork is deleted when its corresponding track has been played. Set this to "yes" to retain all artwork permanently. Warning -- your directory might fill up.
 // pipe_name = "/tmp/shairport-sync-metadata";
 // pipe_timeout = 5000; // wait for this number of milliseconds for a blocked pipe to unblock before giving up
 // socket_address = "226.0.0.1"; // if set to a host name or IP address, UDP packets containing metadata will be sent to this address. May be a multicast address. "socket-port" must be non-zero and "enabled" must be set to yes"
@@ -234,4 +233,5 @@ diagnostics =
 // log_show_time_since_startup = "no"; // set this to yes if you want the time since startup in the debug message -- seconds down to nanoseconds
 // log_show_time_since_last_message = "yes"; // set this to yes if you want the time since the last debug message in the debug message -- seconds down to nanoseconds
 // drop_this_fraction_of_audio_packets = 0.0; // use this to simulate a noisy network where this fraction of UDP packets are lost in transmission. E.g. a value of 0.001 would mean an average of 0.1% of packets are lost, which is actually quite a high figure.
+// retain_cover_art = "no"; // artwork is deleted when its corresponding track has been played. Set this to "yes" to retain all artwork permanently. Warning -- your directory might fill up.
 };

shairport.c

@@ -854,7 +854,7 @@ int parse_options(int argc, char **argv) {
  config.cover_art_cache_dir = (char *)str;
  }
 
- if (config_lookup_string(config.cfg, "metadata.retain_cover_art", &str)) {
+ if (config_lookup_string(config.cfg, "diagnostics.retain_cover_art", &str)) {
  if (strcasecmp(str, "no") == 0)
  config.retain_coverart = 0;
  else if (strcasecmp(str, "yes") == 0)