You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Sutton <pa...@ukweb.com> on 1996/10/22 10:41:54 UTC

Transparant negotiation bug fixes

Koen Holtman has been testing the (partial) transparent negotiation
implement in Apache if #define HOLTMAN is set. He has noticed a few bugs
which need fixing, and provided a patch for some of them (as noted below).
I've also fixed a couple of others, and the patch below is a combination
of these fixes.

The bugs fixed for transparent negotiation are:

   * Should not fiddle Accept header q values (do not set */* to 0.01 etc)
     [Koen Holtman]

   * Now implements definite vs speculative q values
     [Koen Holtman]

   * Should not fiddle language q values for variants with no
     language when some variants have a language

   * Running run_sub_req() in handle_type_map() can result in incorrect
     handler type being applied to selected variant. Replaced with an
     internal_direct().

Paul

*** mod_negotiation.c	Sat Oct 19 20:22:20 1996
--- mod_negotiation.bugfix.c	Sun Oct 20 22:08:54 1996
***************
*** 190,200 ****
      int is_pseudo_html;         /* text/html, *or* the INCLUDES_MAGIC_TYPEs */

      /* Above are all written-once properties of the variant.  The
!      * two fields below are changed during negotiation:
       */

      float level_matched;
      int mime_stars;
  } var_rec;

  /* Something to carry around the state of negotiation (and to keep
--- 191,202 ----
      int is_pseudo_html;         /* text/html, *or* the INCLUDES_MAGIC_TYPEs */

      /* Above are all written-once properties of the variant.  The
!      * three fields below are changed during negotiation:
       */

      float level_matched;
      int mime_stars;
+     int definite;
  } var_rec;

  /* Something to carry around the state of negotiation (and to keep
***************
*** 241,246 ****
--- 243,249 ----
      mime_info->bytes = 0;
      mime_info->lang_index = -1;
      mime_info->mime_stars = 0;
+     mime_info->definite = 1;

      mime_info->charset_quality = 1.0;
      mime_info->type_quality = 1.0;
***************
*** 456,461 ****
--- 459,465 ----
          for (i = 0; i < new->accepts->nelts; ++i)
              if (elts[i].quality < 1.0) new->accept_q = 1;
      }
+     else new->accept_q = 1;

      return new;
  }
***************
*** 947,952 ****
--- 948,956 ----
   * acceptable, but only if no variants with an explicit language
   * are acceptable. The default q value set here is assigned to variants
   * with no language type in set_language_quality().
+  *
+  * Note that if using the transparent negotiation network algorythm,
+  * we don't use this fiddle.
   */

  void set_default_lang_quality(negotiation_state *neg)
***************
*** 954,966 ****
      var_rec *avail_recs = (var_rec *)neg->avail_vars->elts;
      int j;

!     for (j = 0; j < neg->avail_vars->nelts; ++j) {
!         var_rec *variant = &avail_recs[j];
! 	if (variant->content_language && *variant->content_language) {
! 	    neg->default_lang_quality = 0.001;
! 	    return;
  	}
-     }

      neg->default_lang_quality = 1.0;
  }
--- 958,971 ----
      var_rec *avail_recs = (var_rec *)neg->avail_vars->elts;
      int j;

!     if (neg->ua_can_negotiate)
! 	for (j = 0; j < neg->avail_vars->nelts; ++j) {
! 	    var_rec *variant = &avail_recs[j];
! 	    if (variant->content_language && *variant->content_language) {
! 		neg->default_lang_quality = 0.001;
! 		return;
! 	    }
  	}

      neg->default_lang_quality = 1.0;
  }
***************
*** 1071,1076 ****
--- 1076,1082 ----

          variant->lang_quality = best ? best->quality :
  	                     (star ? star->quality : fiddle_q);
+         variant->definite = variant->definite && best;
      }

      /* Now set the old lang_index field */
***************
*** 1119,1124 ****
--- 1125,1131 ----
      int i;
      accept_rec *accept_recs = (accept_rec *)neg->accepts->elts;
      float q = 0.0;
+     int q_definite = 1;

      /* if no Accept: header, leave quality alone (will
       * remain at the default value of 1) */
***************
*** 1167,1174 ****
--- 1174,1184 ----
          if (!neg->accept_q && variant->mime_stars == 1) q = 0.01;
          else if (!neg->accept_q && variant->mime_stars == 2) q = 0.02;
          else q = type->quality;
+
+         q_definite = (variant->mime_stars == 3);
      }
      variant->accept_type_quality = q;
+     variant->definite=variant->definite && q_definite;

      /* if the _best_ quality we got for this variant was 0.0,
       * eliminate it now */
***************
*** 1317,1330 ****
          variant->lang_quality;

  #ifdef NEG_DEBUG
!     fprintf(stderr, "Variant: file=%s type=%s lang=%s acceptq=%1.3f langq=%1.3f typeq=%1.3f q=%1.3f\n",
              variant->file_name ? variant->file_name : "",
              variant->type_name ? variant->type_name : "",
              variant->content_language ? variant->content_language : "",
              variant->accept_type_quality,
              variant->lang_quality,
              variant->type_quality,
!             q
              );
  #endif

--- 1327,1341 ----
          variant->lang_quality;

  #ifdef NEG_DEBUG
!     fprintf(stderr, "Variant: file=%s type=%s lang=%s acceptq=%1.3f langq=%1.3f typeq=%1.3f q=%1.3f definite=%d\n",
              variant->file_name ? variant->file_name : "",
              variant->type_name ? variant->type_name : "",
              variant->content_language ? variant->content_language : "",
              variant->accept_type_quality,
              variant->lang_quality,
              variant->type_quality,
!             q,
!             variant->definite
              );
  #endif

***************
*** 1493,1509 ****
               *   If variant & URI are not neigbors, list_ua or list_os
               *   Else
               *     If UA can do trans neg
!              *        IF best q > 0, choice_ua
!              *        ELSE           list_ua
               *     ELSE
!              *        IF best  > 0, choose_os
!              *        ELSE          list_os (or forward_os on proxy)
               */

              /* assume variant and URI are neigbors (since URI in
               * var map must be in same directory) */

!             algorithm_result = bestq ? na_choice : na_list;
          }
      }

--- 1504,1524 ----
               *   If variant & URI are not neigbors, list_ua or list_os
               *   Else
               *     If UA can do trans neg
!              *        IF best is definite && best q > 0, choice_ua
!              *        ELSE                               list_ua
               *     ELSE
!              *        IF best q > 0, choose_os
!              *        ELSE           list_os (or forward_os on proxy)
               */

              /* assume variant and URI are neigbors (since URI in
               * var map must be in same directory) */

!             if(neg->ua_can_negotiate)
!                algorithm_result = (best && best->definite) && (bestq>0)
!                                        ? na_choice : na_list;
!             else
!                algorithm_result = bestq>0 ? na_choice : na_list;
          }
      }

***************
*** 1750,1758 ****
          if ((res = setup_choice_response(r, neg, best)) != 0)
              return res;

!         /* Run the request. Should we check the output status
!          * for REDIRECT?? */
!         return run_sub_req(best->sub_req);
      }

      /* Make sure caching works - Vary should handle HTTP/1.1, but for
--- 1765,1778 ----
          if ((res = setup_choice_response(r, neg, best)) != 0)
              return res;

! 	/* Doing a run_sub_req() here doesn't work... the
! 	 * sub_req_lookup_file() in setup_choice_response() copies
! 	 * the handler across from the old request, which
! 	 * is 'type-map', so it tries to parse the selected
! 	 * variant as a type-map. Instead, we drop out to
! 	 * the old code, and do an internal_redirect().
! 	 */
! 	/* return run_sub_req(best->sub_req); */
      }

      /* Make sure caching works - Vary should handle HTTP/1.1, but for


Re: Transparant negotiation bug fixes

Posted by Brian Behlendorf <br...@organic.com>.
I'm attempting to merge all these into the current CVS tree.

Weird problems:

On Tue, 22 Oct 1996, Paul Sutton wrote:
> *** mod_negotiation.c	Sat Oct 19 20:22:20 1996
> --- mod_negotiation.bugfix.c	Sun Oct 20 22:08:54 1996
> ***************
> *** 456,461 ****
> --- 459,465 ----
>           for (i = 0; i < new->accepts->nelts; ++i)
>               if (elts[i].quality < 1.0) new->accept_q = 1;
>       }
> +     else new->accept_q = 1;
> 
>       return new;
>   }

This "else" doesn't appear to be tied to an "if" - the block before this is a
while statement.  Wha?

The rest looks fine.  I had to apply a lot by hand however.  The current diff
is sent here as an attachment.

	Brian