Merge pull request chef#10 from opscode/sf/ipv6

Add support for bracketed ipv6 address literals when generating URLs

.gitignore

@@ -6,3 +6,4 @@ _build
 ebin/*.beam
 ebin/*.app
 .eunit
+/.concrete/DEV_MODE

Makefile

@@ -1,49 +1,28 @@
-REBAR=$(shell which rebar)
-ifeq ($(REBAR),)
-	$(error "Rebar not available on this system")
+# This Makefile written by concrete
+#
+# {concrete_makefile_version, 1}
+#
+# Use this to override concrete's default dialyzer options of
+# -Wunderspecs
+# DIALYZER_OPTS = ...
+
+# List dependencies that you do NOT want to be included in the
+# dialyzer PLT for the project here.  Typically, you would list a
+# dependency here if it isn't spec'd well and doesn't play nice with
+# dialyzer or otherwise mucks things up.
+#
+# DIALYZER_SKIP_DEPS = bad_dep_1 \
+#                      bad_dep_2
+
+# If you want to add dependencies to the default "all" target provided
+# by concrete, add them here (along with make rules to build them if needed)
+# ALL_HOOK = ...
+
+concrete_rules_file = $(wildcard concrete.mk)
+ifeq ($(concrete_rules_file),concrete.mk)
+    include concrete.mk
+else
+    all:
+	@echo "ERROR: missing concrete.mk"
+	@echo "  run: concrete update"
 endif
-
-APPDIR=$(CURDIR)
-SRCDIR=$(APPDIR)/src
-EBINDIR=$(APPDIR)/ebin
-
-PLT_DIR=$(CURDIR)/.plt
-DEPS_PLT=$(PLT_DIR)/mini_s3
-DIALYZER_DEPS=ibrowse
-
-ERLPATH=-pa $(EBINDIR) -pa $(APPDIR)/deps/*/ebin
-
-.PHONY=all clean_plt dialyzer typer compile clean distclean test
-
-all: compile test dialyzer
-
-deps:
-	$(REBAR) get-deps
-
-$(PLT_DIR):
-	mkdir -p $(PLT_DIR)
-
-$(DEPS_PLT): $(PLT_DIR)
-	dialyzer --build_plt --output_plt $(DEPS_PLT) \
-		$(ERLPATH) --apps $(DIALYZER_DEPS)
-
-clean_plt:
-	rm -rf $(PLT_DIR)
-
-dialyzer: $(DEPS_PLT)
-	@dialyzer -Wrace_conditions -Wunderspecs \
-        --plts ~/.dialyzer_plt $(DEPS_PLT) -r $(EBINDIR)
-
-typer: compile $(PLT)
-	typer --plt $(PLT) -r $(SRCDIR)
-
-compile:
-	$(REBAR) compile
-
-test:
-	$(REBAR) skip_deps=true eunit
-
-clean:
-	$(REBAR) clean
-
-distclean: clean clean_plt

concrete.mk

@@ -0,0 +1,92 @@
+DEPS ?= $(CURDIR)/deps
+
+DIALYZER_OPTS ?= -Wunderspecs
+
+# Find all the deps the project has by searching the deps dir
+ALL_DEPS = $(notdir $(wildcard deps/*))
+# Create a list of deps that should be used by dialyzer by doing a
+# complement on the sets
+DEPS_LIST = $(filter-out $(DIALYZER_SKIP_DEPS), $(ALL_DEPS))
+# Create the path structure from the dep names
+# so dialyzer can find the .beam files in the ebin dir
+# This list is then used by dialyzer in creating the local PLT
+DIALYZER_DEPS = $(foreach dep,$(DEPS_LIST),deps/$(dep)/ebin)
+
+DEPS_PLT = deps.plt
+
+ERLANG_DIALYZER_APPS = asn1 \
+                       compiler \
+                       crypto \
+                       edoc \
+                       erts \
+                       eunit \
+                       gs \
+                       hipe \
+                       inets \
+                       kernel \
+                       mnesia \
+                       observer \
+                       public_key \
+                       runtime_tools \
+                       ssl \
+                       stdlib \
+                       syntax_tools \
+                       tools \
+                       webtool \
+                       xmerl
+
+all: .concrete/DEV_MODE compile eunit dialyzer $(ALL_HOOK)
+
+.concrete/DEV_MODE:
+	@mkdir -p .concrete
+	@touch $@
+
+# Clean ebin and .eunit of this project
+clean:
+	@rebar clean skip_deps=true
+
+# Clean this project and all deps
+allclean:
+	@rebar clean
+
+compile: $(DEPS)
+	@rebar compile
+
+$(DEPS):
+	@rebar get-deps
+
+# Full clean and removal of all deps. Remove deps first to avoid
+# wasted effort of cleaning deps before nuking them.
+distclean:
+	@rm -rf deps $(DEPS_PLT)
+	@rebar clean
+
+eunit:
+	@rebar skip_deps=true eunit
+
+test: eunit
+
+# Only include local PLT if we have deps that we are going to analyze
+ifeq ($(strip $(DIALYZER_DEPS)),)
+dialyzer: ~/.dialyzer_plt
+	@dialyzer $(DIALYZER_OPTS) -r ebin
+else
+dialyzer: ~/.dialyzer_plt $(DEPS_PLT)
+	@dialyzer $(DIALYZER_OPTS) --plts ~/.dialyzer_plt $(DEPS_PLT) -r ebin
+
+$(DEPS_PLT):
+	@dialyzer --build_plt $(DIALYZER_DEPS) --output_plt $(DEPS_PLT)
+endif
+
+~/.dialyzer_plt:
+	@echo "ERROR: Missing ~/.dialyzer_plt. Please wait while a new PLT is compiled."
+	dialyzer --build_plt --apps $(ERLANG_DIALYZER_APPS)
+	@echo "now try your build again"
+
+doc:
+	@rebar doc skip_deps=true
+
+tags:
+	find src deps -name "*.[he]rl" -print | etags -
+
+.PHONY: all compile eunit test dialyzer clean allclean distclean doc tags

rebar.config.script

@@ -0,0 +1,90 @@
+%% -*- mode: erlang -*-
+%% -*- tab-width: 4;erlang-indent-level: 4;indent-tabs-mode: nil -*-
+%% ex: ts=4 sw=4 ft=erlang et
+
+%% rebar.config.script GENERATED BY concrete
+%%
+%% YOU SHOULDN'T NEED TO EDIT THIS FILE
+%%
+{concrete_rebar_script_version, 1}.
+
+%% We need the following helper function to merge dev only options
+%% into the values provided by rebar.config.
+
+%% Merge the list values in `ToAdd' into the list found at key `Key'
+%% in proplist `C'. Don't duplicate items. New Items are added to the
+%% front of existing items. It is an error if the value at `Key' is
+%% not a list in `C'.
+MergeConfig = fun({Key, ToAdd}, C) ->
+                      case lists:keyfind(Key, 1, C) of
+                          false ->
+                              lists:keystore(Key, 1, C, {Key, ToAdd});
+                          {Key, List} when is_list(List) ->
+                              %% remove items in ToAdd already in List
+                              ToAdd1 = [ I || I <- ToAdd, not lists:member(I, List) ],
+                              lists:keystore(Key, 1, C, {Key, ToAdd1 ++ List })
+                      end
+              end.
+
+%% -- Add development only options if we are a top-level build  --
+%%
+%% If a file named `.concrete/DEV_MODE' exists, we assume we are a
+%% top-level build (not being built as a dep of another project). We
+%% add the deps from dev_only_deps defined in rebar.config, add
+%% concrete as a dep, and define the compiler macro DEV_ONLY.
+
+%% This macro can be used to conditionally enable code (e.g. tests)
+%% that depend on development only dependencies.
+ErlOpts = {erl_opts, [
+    {d, 'DEV_ONLY'}
+]},
+
+%% Development only dependencies can be specified in the main
+%% rebar.config. This file should not need to be edited directly.
+DevOnlyDeps = case lists:keyfind(dev_only_deps, 1, CONFIG) of
+                  false ->
+                      [];
+                  {dev_only_deps, DOD} ->
+                      DOD
+              end,
+
+EdownMe =
+fun(C) ->
+        %% Unless rebar.config contains `{use_edown, false}', we'll
+        %% wire it up. However, we'll only add the required edoc_opts
+        %% if none have been provided.
+        case proplists:get_value(use_edown, C) of
+            false ->
+                C;
+            _ ->
+                %% only add edoc_opts if none are present
+                C1 = case lists:keymember(edoc_opts, 1, C) of
+                         true ->
+                             C;
+                         false ->
+                             MergeConfig({edoc_opts, [{doclet, edown_doclet}]}, C)
+                     end,
+                MergeConfig({deps,
+                             [{edown, ".*",
+                               {git, "git://github.com/seth/edown.git",
+                                {branch, "master"}}}]},
+                            C1)
+        end
+end,
+
+Deps = {deps, DevOnlyDeps},
+
+ConfigPath = filename:dirname(SCRIPT),
+DevMarker = filename:join([ConfigPath, ".concrete/DEV_MODE"]),
+
+case filelib:is_file(DevMarker) of
+    true ->
+        lists:foldl(fun(I, C) -> MergeConfig(I, C) end,
+                    EdownMe(CONFIG), [Deps, ErlOpts]);
+    false ->
+        %% If the .concrete/ marker is not present, this script simply
+        %% returns the config specified in rebar.config. This will be
+        %% the behavior when the project is built as a dependency of
+        %% another project.
+        CONFIG
+end.

src/mini_s3.erl

@@ -63,6 +63,10 @@
          make_authorization/10,
          make_signed_url_authorization/5]).
 
+-ifdef(TEST).
+-compile([export_all]).
+-endif.
+
 -include("internal.hrl").
 -include_lib("xmerl/include/xmerl.hrl").
 -include_lib("eunit/include/eunit.hrl").
@@ -413,7 +417,7 @@ if_not_empty(_, Value) ->
 -spec format_s3_uri(config(), string()) -> string().
 format_s3_uri(#config{s3_url=S3Url, bucket_access_type=BAccessType}, Host) ->
     {ok,{Protocol,UserInfo,Domain,Port,_Uri,_QueryString}} =
-        http_uri:parse(S3Url),
+        http_uri:parse(S3Url, [{ipv6_host_with_brackets, true}]),
     case BAccessType of
         virtual_hosted ->
             lists:flatten([erlang:atom_to_list(Protocol), "://",

test/mini_s3_tests.erl

@@ -0,0 +1,50 @@
+%% -*- erlang-indent-level: 4;indent-tabs-mode: nil; fill-column: 92 -*-
+%% ex: ts=4 sw=4 et
+%% Copyright 2013 Opscode, Inc. All Rights Reserved.
+%%
+%% This file is provided to you under the Apache License,
+%% Version 2.0 (the "License"); you may not use this file
+%% except in compliance with the License.  You may obtain
+%% a copy of the License at
+%%
+%%   http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing,
+%% software distributed under the License is distributed on an
+%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+%% KIND, either express or implied.  See the License for the
+%% specific language governing permissions and limitations
+%% under the License.
+%%
+-module(mini_s3_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+-include("../src/internal.hrl").
+
+format_s3_uri_test_() ->
+    Config = fun(Url, Type) ->
+                     #config{s3_url = Url, bucket_access_type = Type}
+             end,
+    Tests = [
+             %% hostname
+             {"https://my-aws.me.com", virtual_hosted, "https://bucket.my-aws.me.com:443"},
+             {"https://my-aws.me.com", path, "https://my-aws.me.com:443/bucket"},
+
+             %% ipv4
+             {"https://192.168.12.13", path, "https://192.168.12.13:443/bucket"},
+
+             %% ipv6
+             {"https://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]", path,
+              "https://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:443/bucket"},
+
+             %% These tests document current behavior. Using
+             %% virtual_hosted with an IP address does not make sense,
+             %% but leaving as-is for now to avoid adding the
+             %% is_it_an_ip_or_a_name code.
+             {"https://192.168.12.13", virtual_hosted, "https://bucket.192.168.12.13:443"},
+
+             {"https://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]", virtual_hosted,
+              "https://bucket.[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:443"}
+            ],
+    [ ?_assertEqual(Expect, mini_s3:format_s3_uri(Config(Url, Type), "bucket"))
+      || {Url, Type, Expect} <- Tests ].