to do: ========== correctness: * Broken servers: The current scheme (automagically retry) is complex and basically a little bit crackrock. The retry/ignore algorithm sucks and is broken. It is probably better to just add a new LAC_HTTP_ERROR_BUGGY_PIPELINING indicating that the server-side implementation of pipelining is buggy. To make this less annoying for apps, there should be a higher level wrapper that fixes stuff up. This should sit above HttpTransport * Need to handle POLLERR on the dns file descriptor. We can somehow get connrefused. LacWatch code should also be fixed to it doesn't spin whent it gets a POLLERR that noone cares about. * TLS - Implement the rest of the connection methods - Make a wrapper Connection class that can be either an SSL or a TCP connection. - Move shared code to the wrapper - Make sure TLSConnection sends events at the right time. (Ie., never directly in response to a method call). Make sure it's properly reentrant. * What is it we need the shutdown method for on connections? * SIGPIPE? - ignore it around calls to send()? Problem: Other threads might not want the signal ignored. This race condition may be acceptable Another possibility is to just add a lac_sigpipe_ignore() and punt to the applications * thread-safeness (big lock?) (showstopper for 1.0). Note: lac_activity_cancel() from another thread. This could get hairy :-/ * Periodically stat /etc/resolv.conf and reload it if it has changed. (Though people will probably still think they have to restart even if they don't). * consider activity = lac_begin_activity (timeout); ...; lac_activity_wait(activity)/lac_activity_cancel(); lac_end_activity(activity); within "..." you can call lac_activity_cancel(); * relative URI's - showstopper for 1.0 The code from GnomeVFS is probably not good enough (it doesn't handle relative uri's that start with "..". Such uri's are broken, but fairly common) * we don't interprete resolv.conf correctly. Some options are simply ignored. * Do proper URI escaping. (There is an escape_spaces() hack in lachttp.c, but we should do better). * A server has been spotted that returns "HTTP/00 Ok" as the status line. What do we do? - Declare it broken and retry. - Accept it based on "Ok". * Broken-ness with _wait() functions that get the callback immediately. This causes them to wait forever in g_main_loop_run(). * Make sure local names are handled. (/etc/host.conf and /etc/hosts) misc: * Use gchar everywhere instead guint/guchar - all they get us is annoying casts. Well, LacDNS relies on unsigned chars in some places. * Think the LacByteQueue usage through. Right now there is still some unnecessary copying because you can't push data back into a connection. * Consider changing LacByteQueue name to LacBuffer * Probably use gsize instead of guint in some places, such as lac_send * consider moving complexity from nameserver to query, ie. have the nameserver have one response function used for all replies. The nameserver would not keep the map between queries and reply functions. We would have to have that map in the query file, but it may not be that bad since we already have a table of outstanding queries there. * need a real regression test suite - including a list of sites known to be broken in various ways. * get rid of CacheRecord->type * Cleanup of the DNS module - "_LacDns" prefix internal-but-exported symbols * #ifdef LAC_DEBUG_ENABLED around debug code * debug messaging framework * GObjectify stuff - Is this worth it? Plus, GObject is basically unmaintained ... * some functions in lacprivate.c report errors with g_warning * error messages should not be \n-terminated * The callbacks should probably have 'data' parameters instead of relying on 'get_data()' methods. In non-trivial applications you are going to want the data anyway. performance: * figure out when exactly the write buffer should be flushed in the http module. - use "LoadGroup" and ask the application to call flush() when it won't send anymore requests for a while - add a timeout of say 200 ms so that a request wont stay longer than this timeout in the send queue - Who cares? * When an answer for one DNS query arrives, go through all outstanding queries to see if the information was useful there? This could be expensive if we have many outstanding queries. Note that this will not get rid of the "use existing" code, because we still want to avoid sending any messages to the server if we can. consistency: * generally: - foo_get_bar() means "whatever is returned is owned by the library - you must copy or ref it". This is used both with return values and with return-by-callback. However with an out parameter, the user is generally responsible for freeing. - foo_wait_for_bar() means "lac will start a main loop" This is used only with return values - foo_lookup_bar() means whatever is returned is owned by the library - you must copy or ref it. Used with return-by-callback. - foo_new() means "you must free whatever is returned" - foo_new_lookup_from_bar() means "a new foo is created, you must free it" * len -> length error handling: * make the err->messages more helpful * better and more error codes socket(): Irix: EPROTONOSUPPORT, EMFILE, ENFILE, EACCES, ENOBUFS Solaris: EPROTONOSUPPORT, EMFILE, ENOMEM, ENOSR, EACCES Glibc: EPROTONOSUPPORT, EMFILE, ENFILE, EACCES, ENOBUFS, ENOMEM, EINVAL HP-UX: EAFNOSUPPORT, EHOSTDOWN, EINVAL, EMFILE, ENFILE, ENOBUFS, EPROTONOSUPPORT, EPROTOTYPE, ESOCKTNOSUPPORT, ETIMEDOUT connect: Solaris: EACCES, EADDRINUSE, EADDRNOTAVIL, EAFNOSUPPORT, EALREADY, EBADF, ECONNREFUSED, EINPROGRESS, EINTR, EINVAL, EIO, EISCONN, ELOOP, ENETUNREACH, ENOENT, ENOSR, ENXIO, ETIMEDOUT, EWOULDBLOCK, ENOTDIR ENOTSOCK, EPROTOTYPE Irix: EBADF, ENOTSOCK, EADDRNOTAVAIL, EAFNOSUPPORT, EISCONN, ETIMEDOUT, ECONNREFUSED, ENETUNREACH, EADDRINUSE, EFAULT, EINPROGRESS, EALREADY HP-UX: EADDRINUSE, EADDRNOTAVAIL, EAFNOSUPPORT, EALREADY, EBADF, ECONNREFUSED, EFAULT, EINPROGRESS, EINTR, EINVAL, EISCONN, ENETDOWN, ENETUNREACH, ENOBUFS, ENOBUFS, ENODEV, ENOSPC, ENOTSOCK, EOPNOTSUPP, ETIMEDOUT Glibc: EBADF, EFAULT, ENOTSOCK, EISCONN, ECONNREFUSED, ETIMEDOUT, ENETUNREACH, EADDRINUSE, EINPROGRESS, EALREADY, EAFNOSUPPORT, EACCES features: * We should handle HTTP redirects - Probably add a new "RedirectNotify" event - Be able to turn it off for a request. An app. may want to handle it differently * IPv6 - not for 1.0, but must make sure it can be added tentative api: bool lac_address_is_ip6(); Assumption: the LacAddress will never need to cover more than IP4 and IP6, at least not binary compatibly. - consider for 1.0 to make the ipv4 specific API conditional on !LAC_IPV6_UNSAFE. Later on this API will g_return_if_fail() when faced with an IP6 address. - Another idea is to just add the is_ip6() call and have it always return TRUE for 1.0. - Another idea is to just remove the get_a_b_c_d() call, or rename it lac_address_ip4_get_a_b_c_d(). - get_a_b_c_d() *is* unfortunately useful for protocols involving sending addresses. Maybe add a const char * address_serialize (LacAddress *, int &length); The returned string would be valid for as long as the address is valid. Maybe also address_new_from_serialized() - Note that sockets are different depending on the protocol used. Ie., s = socket (AF_INET6, SOCK_STREAM, 0); vs. s = socket (AF_INET, SOCK_STREAM, 0); so just virtualizing the LacAddress may not be enough. Maybe lac_socket_tcp6() is sufficient. Need to test if an AF_INET6 socket can be used for IP4 communication. If so, we can get away with just virtualizing addresses, if not, we probably lose. According to RFC 3493 it does look like you can use an IP6 socket to communicate with IP4 nodes. * https/ssl support, at least make is possible later * proxy support in http module - At least make sure the API doesn't prevent us adding it naturally later maybe we just need lac_http_request_set_proxy (...); * http cache - We should probably virtualize the read() and write() so file/persistent operations can be changed to use eg. async io on linux. possible api: we already have lac_request_dispatch() lac_http_cache_new (directory, read, write, ...); lac_http_cache_dispatch_request (LacHttpRequest *req, gboolean reload); - How abstract should the read() and write() be? - On the other hand, maybe the library should know about different ways of structuring i/o - Cache does have to live in library - otherwise it can't deal with half-done requests and multiple simultaneous requests for the same object. * cookie support (see RFC 2109) activity: Use cases: 1 Dialog with a timeout and a cancel button and several network activities that all must be completed before the timeout. 2 Just waiting for a DNS lookup (can't go on without it) 3 Tell me when the lookup arrives ad 1: boolean timeout (void) { update progressbar; if (timed out) { cancel all activityies; destroy_dialog; timeout_id = 0; return FALSE; } } activity1 = start_activity_1 (..., f1, data1); ...; activity_k = start_activity_k (..., f_k, data_k); struct { activities, dialog, dialog->progressbar } data; lac_timeout (100, timeout, &data); if (gtk_dialog_show (dialog) == CANCEL) { lac_activity_cancel (activity1, ..., activityk); } if (timeout_id) g_timeout_remove (timeout_id); ad 2: lac_gethostbyname(); *** different scheme **** activity = lac_create_activity (); lac_various_stuff (.., activity); lac_more_stuff (, ... activity); lac_activity_cancel (activity); // cancels the whole thing lac_activity_wait (activity, timeout); // wait until they all // have completed drawback here is that all calls must take an activity parameter. Programs that don't know about "activity" will have to pass NULL. Another possibility is LacFuture lac_address_new_from_name_future (); lac_future_free (LacFuture); lac_future_resolve_string() lac_future_resolve_address() lac_future_resolve_pointer() gboolean lac_future_in_progress() gboolean lac_future_available() gboolean lac_future_canceled() lac_future_wait_for_one (timeout, future, ..); // wait for at least one of the futures // to become canceled, available or destroyed lac_future_wait_for_all (timeout, ...); lac_future_set_notification (future, callback, data); /* possibly */ Future lac_future_combine_or (future1, future2, ...); // create new future that becomes // available when one of the futures // are available Future lac_future_comibne_and (future1, future2, ...); enum FutureStates { FUTURE_IN_PROGRESS, FUTURE_CANCELED, FUTURE_AVAILABLE, } My current thinking is: get rid of all the "Activity" stuff, then (maybe post 1.0): add the LacFuture stuff and reimplement the existing methods with that. LacFuture implementation note: - it is illegal to destroy a future that you are waiting on The idiom is create futures install timeout that possibly cancels futures. Timeout can also update progress bars etc. future = show_dialog (); wait for futures if (future_is_available (future1)) { // you can assume everything is available } destroy futures; It should probably be possible to create futures yourself. And they should probably just return a gpointer. lac_future_new (); lac_future_make_available (future, gpointer); lac_future_set_cancel_notify (future, cancel_func, data); response () { Future *future; lac_future_make_available (future, GTK_RESPONSE_NO); } Future show_dialog () { ...; future = future_new(); gtk_widget_show(); signal_connect_connect (response, yada, yada, future); } The "Future" object should probably go into glib so it could be used for gtk+ dialogs and in gdk. "application API": GFuture gtk_dialog_run (); ... void g_future_cancel (GFuture *future); void g_future_wait (GFuture *future, ...); void g_future_destroy (GFuture *future); gboolean g_future_is_available (GFuture *future); gpointer g_future_get_result (GFuture *future); "library API": lac_future_new (); lac_future_make_available (GFuture *future); lac_future_set documentation: * write it * always have ref()/unref() pairs and perhaps a close()/shutdown() on objects. - easier bindings for languages with gc - it is CRACK to use ref_count as any indicator of how many clients are interested in events. * LacActivity: This is perfectly okay and guaranteed to work: hmm, it doesn't work: what if the returned activity is NULL? typedef struct LookupData LookupData; struct LookupData { int timeout_id; LacActivity activity; gboolean canceled; }; static gboolean timeout_handler (gpointer data) { LookupData *lookup_data = data; lookup_data->canceled = TRUE; g_source_remove (lookup_data->timeout_id); lac_activity_cancel (lookup_data->activity); return FALSE; } int main() { LookupData data; LacAddress *addr; data.canceled = FALSE; data.timeout = g_timeout_add (1000, timeout_handler, &data) addr = lac_address_new_from_name_wait ( "www.cocacola.com", &data.activity, NULL); if (data.canceled) { ...; } else if (addr) { ...; } } Already done: ============== misc: * Consider making the LacActivity cookies out parameters * never emit events from a constructor. apps will get confused if they get an event before the constructor returns. add an idle handler instead. [May already be done, need to check] * move a lot of #includes from lac.h to lacprivate.h * rename lacdns-manager.[ch] -> lacdns-query.[ch]. * format functions correctly * Major cleanup of the DNS module - Split into modules: main, cache, server, messages - separate header files. - Figure out exactly what needs to be exported from each module. - one big header-file (not done) Probably do this one module at a time. * make lac_address_new_from_name() use lac_dns_get_addresses * don't expose of address internals. Use a _lac_get_internals() function instead. * get rid of UNIX sockets completely? Rationale: they will probably complicate the API significantly, and it can be argued that they do not belong in a network library since they are really a form of IPC on the same machine. If we are going to include them, it may make sense to consider a more object-oriented approach to the API. On the other hand, getting rid of them will probably mean they can't be added later. correctness: * Fix problem where we go into infinite loop if the server closes the connection before we send anything. (showstopper for 1.0) - www.skolekom.dk used to be like that (problem is in transport_handle_close) [I think this is fixed, not completely sure] * We get 'broken server' too often * adobe.com is not completely fixed - answer_timeout is apparently somewhat broken. It should only be installed when the transport is actually pipelining, and it should only fire once. Also, hosts should ignore broken notifies when the host is already broken. - There is a similar problem with art.gnome.org, only here the server sends an RST when it sees a pipeline * We don't work with www.adobe.com/main.html also, art.gnome.org sends RST when it sees a pipeline Problems (alle of these only happens with pipelining). - Sometimes we get malformed replies to the very first request. strace confirms that these are genuinely malformed. There is not much we can do about that except report the error. (This happens rarely, and only immediately after a pipelined attempt). - Sometimes the server just stops responding in the middle of a request. We can detect this with the answer_timeout. When that timeout fires, there _is_ a current request and a lot of requests in progress. We can reschedule the in-progress requests and emit a "broken_server_notify" , but what about the "current" request? - add a new BROKEN_SERVER_PLEASE_RETRY error message. - Apps will probably get it wrong - Add a new RESTARTING event meaning that the app is supposed to throw away what is has read so far - Apps will probably get it wrong. - let it be - The *user*/app will eventually cancel it. - Definitely suboptimal - silently retry, and ignore as many bytes as we have already seen so far. - It sucks. - It is broken (because stuff may have been cached, and the next request might return something different) - But it is probably the best we can do * Strategy for dealing with broken servers in general. - We are optimistic. When we see a broken server, resend everything on separate connections - what if server is so broken that it *never* responds to any requests in a pipeline? (jp.dk) - just don't work with these? - don't hear anything for k * RTT, close the connection and retry? Also maintain a list of servers known to be broken. That way we will be able to disable pipelining for those in the case where we initially send only one request. - We are pessimistic. Only pipeline when we *know* the server is not broken. - significantly reduce the utility of pipelining * Fix problem with www.jp.dk. Apparently it has an http 1.0 cache in front of it that accepts http 1.0 keep-alive, but doesn't understand pipelining. It seems that when it gets more than one request, it ignores all of them. Find out if this cache (CacheFlow 725?) is special, or if many http 1.0 implementations behave like this. Note, http://www.cocacola.com/ is an HTTP 1.0 server that fully understands pipelining with connection: keep-alive. * Fix problems with Enterprise 3.6 and the http module. eg. www.sgi.com runs enterprise 3.6. (Enterprise 3.6 is known to be broken. See http://www.research.att.com/~bala/papers/usits01.ps.gz It is probably best to just not pipeline with E3.6. There are very few of them anyway). * it is definitely wrong that domain_name_hash() is case sensitive while domain_name_equal() isn't. Making domain_name_hash() case insensitive would solve it, but rethink case-sensitivity instead. - assume domain name are case sensitive. - would probably mean a lot extra lookups if the user actually asks for the same name in different capitalizations. - some extra entries in the cache. - probably broken: (If we ask for "blah.com", we may get the answer "BLAH.COM". What will we do about that?) - would make UTF-8 just work (?) - just make domain_name_hash() case insensitive - slightly unclean, I think. - utf-8 problem. Is utf-8 actually going to be standard? [Ended up just making domain_name_hash() case insensitive] * make sure we don't emit any events after the user has called lac_connection_close() * www.cocacola.com is an http 1.0 server. if we pipeline a lot of requests to it, it will respond to them, so when we decide that a server can't pipeline due to being 1.0, we should wait until it _actually_ closes the connection before reporting to the host. * well, if we get a "Connection: keep-alive", then we should keep the connection alive * a response from an 1.0 server is generally terminated by closing the connection, unless we have reason to believe otherwise * validate all input to the dns implentation as UTF8 (- right?) - probably not. No, the dns system allows arbitrary data for domain names * "http-test http:/cmdrtaco.net" produces something with "localhost" * use lac_dns_get_name() in lac_address_get_hostname() * in lac_address_new_from_new(), if dns_query() calls back immediately, the activity will be invalid. What do we do?? * It seems sometimes the answer timeout fires after the transport has been destroyed. performance: * With an unstable network connection, DNS packages will often get lost. We should probably be a little more aggressive about requerying. For example resending every .5 second under the assumption that with a well-working network, the response will usually arrive well within .5 seconds. * add a timeout implementation? [nah, fix the one in glib instead] - better scalablity - perhaps more convenient - initial implementation can be very simple in terms of g_timeout_add() API: /* * Timeout */ typedef struct _LacTimeout LacTimeout; LacTimeout *lac_timeout_new (guint interval, GSourceFunc callback, gpointer data); void lac_timeout_set_interval (LacTimeout *timeout, guint interval); guint lac_timeout_get_interval (LacTimeout *timeout); void lac_timeout_disable (LacTimeout *timeout); void lac_timeout_enable (LacTimeout *timeout); void lac_timeout_destroy (LacTimeout *timeout); * LacCowString ([not worth it]) - do a simple copy-on-write string on contiguous memory pro: simple, easy to get c-string con: insert in the middle of it is inefficient - how much data in an xml/html document is markup and how much is character data? - we risk doing a lot if copying during parsing of xml if eg a begin tag spans more than one chunk of data - even if we get the cow string into glib and get gdkpixbuf to use it, images may need to a lot of uncompressing anyway so that all of the data will be copied anyway. - actually, memcpy() is probably not really a bottleneck if do aggressive caching and reuse images whenever possible. - since we have "const char *" and a dummy reserved pointer in events, we can do this later without breaking anything. possible API: /* * Copy-on-write string */ typedef struct _LacCowString LacCowString; LacCowString *lac_cow_string_new (const gchar *init, gsize len); LacCowString *lac_cow_string_copy (const LacCowString *cow_string); void lac_cow_string_free (LacCowString *cow_string); gboolean lac_cow_string_equal (const LacCowString *cow_string1, const LacCowString *cow_string2); guint lac_cow_string_hash (const LacCowString *cow_string); gint lac_cow_string_compare (const LacCowString *cow_string1, const LacCowString *cow_string2); LacCowString *lac_cow_string_truncate (LacCowString *cow_string, gsize size); LacCowString *lac_cow_string_prepend (LacCowString *cow_string, const gchar *val, gsize len); LacCowString *lac_cow_string_insert (LacCowString *cow_string, gsize pos, const gchar *val, gsize len); LacCowString *lac_cow_string_append (LacCowString *cow_string, const gchar *val, gsize len); LacCowString *lac_cow_string_erase (LacCowString *cow_string, gsize pos, gsize len); LacCowString *lac_cow_string_concatenate (LacCowString *cow_string1, ...); * limit number of outstanding dns queries? Yes. * sockets are almost always writable, so in general we should avoid poll()ing just to write. Instead, just send(), and only call poll() if we get a WOULDBLOCK. - This is mosly done, I think * dispatch functions, like blah_do_read() and blah_do_write() should not run for more than, say, 10 ms. This should normally be enough to empty the buffers while not starving the rest of the application - this is probably not worth it. Generally, a while(read()) will complete in << 10 ms. - When COWStrings are there, maybe simply read into one big cow string and then return that. If the cow string starts its life at size 8192, then we'll almost never have to expand it. - Or generate a list of cowstrings as we read(), and then call the callback several times. consistency: * be consistent about "str" vs. "string" - what does glib do? (a little of both) I'll go with string * be consistent about whether len comes before data or vice versa - data comes first followed by length. This is consistent with unix, glib and everything else * be consistent about whether stuff returned through callbacks is owned by the user or the library - probably best to make it be owned by the library, because in some cases we have to return compound structures that are difficult for the app to free - probably best to make addresses copyable by value and get rid of ref counting on them * fix instances of if (err->code == ...) features: * re-add gethostby{name,addr}() with a recursive main loop. * LacCowString - do a complicated "smart string" pro: insert in the middle will be more efficient con: more complicated, get_c_string is difficult there is considerable overhead for the rest libxml may need to do some rewriting, ie. to convert to utf8. This rewriting will have to be done under any circumstances. the "smart string" is definitely not worth it. the simple _might_ be worth it, but error handling: * Use g_error_matches() everywhere. * rename lacprivate.c -> lacprimitives.c? * should we make sure *everything* is emitted from the main thread. ie., idle_add() stuff that we would otherwise do directly? - Probably not, because the application is in a much better position to make scheduling decisions * we should never call a callback when the we are in an inconsistent state. Strategy for dealing with reentrancy: Queue up callbacks, then run the queue at end of all public entries. For the purposes of reentrancy, functions that we added to the mainloop should be considered public. * There is a reentrancy problem in lacdns-query. The current strategy of putting uncachable records in the cache, then deleting them later doesn't work because the uncachable records are not removed when we call the user. * the resend timeout should be shorter than the QUERY timeout, so that if the answer to a previous query arrives after we have sent a new query, then we should use it. * DNS API - Lose lac_dns_* functions? Fold them into the lac_address_* namespace? That would probably make the API smaller and more consistent. OTOH, the lac_address_* names would become longer. It would be possible to add lac_dns_* functions again later, eg. for mx and other dns stuff. Is there an IPv6 issue with this? /* address_new_from_name_wait */ /* address_wait_for_name */ /* address_lookup_name */ /* address_new_from_name_all */ - Do we need user settable timeouts on a request? Does Windows/.NET have this? - cancelation of DNS lookups? make them return an int and have a new call lac_async_cancel (int async_id); Both could be handled by int lac_address_get_from_name (name, func, data); int lac_address_get_all_from_name (name, func, data); void lac_address_cancel_lookup (int id); LacAddress *lac_address_new_from_name_wait (name); void lac_address_nwe_all_from_name (name); * adobe.com is not completely fixed sometimes it returns malformed responses after we have detected that it choked. The answer here is probably "Well, tough." There is only so much brokenness we can deal with. * Improve HTTP/0.9 recognitiion. Algorithm: - if first four letters are not HTTP, 0.9 - else if next letters are correct 1.0 or 1.1 1.0 or 1.1 - else 1.0 * We send out broken DNS messages if we are called with a very long name * In general when we use callbacks, always let the main loop call back, if necessarily by using an idle hander. (It is important that the user can count on the callback not having been called when the function returns) - more precisely: functions with both a return value and a callback must call callback in idle (ie. after the function has returned) - No, actually not. It is even more important that things like dns callbacks are processed immediately, because the processing will often just consist in "socket(); connect()", which will make *someone else* do work, which is good.