Server class API issue?

[Paul Stoffregen]

Recently I've been working on improving the Ethernet library
performance. Along the way, I've been mystified by what seems like a
huge hole in the Server/EthernetServer class. Hopefully I've just
misunderstood something?

As nearly as I can tell, EthernetServer can only implement protocols
like HTTP where the conversation begins by the client sending data first.

For example, imagine trying to implement a Telnet server. Hopefully we
can focus on the Server API without getting bogged down in Telnet's
plaintext password security. When a client connects, the server is
supposed to transmit a welcome message and the login prompt. Then the
client sends their username in response to the prompt, and then a
password prompt is sent, and so on...

The problem is the Server.available() only returns a valid Client object
after data has been received. That works great for HTTP where the
client begins the conversation by sending a request to the server, and
it's really convenient for a one-client-at-a-time web server. But how
is Arduino's API supposed to be used to implement servers where the
conversation is supposed to begin by the server sending something like a
welcome message?

Have I just overlooked or misunderstood something? Or does Arduino's
API lack any way to implement such servers?

[Peter Feerick]

Hi Paul,

Probably a dumb question here (as I've only just woken up and haven't had my morning coffee yet)... but how would server.available() be able to return if there is no client connected yet? telnet should still be reactive - i.e. client connects, server sends welcome message.

How close is this code[1] to creating functioning (and suitably rudimentary) telnet server? I have to admit that this [2] code looks a lot cleaner.

Pete

[1] http://forum.arduino.cc/index.php?topic=8533.0

[2] http://blog.bouni.de/blog/2012/06/25/a-arduino-telnet-server/

Peter Feerick
BIT, BLDes CQU

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

[follower]

On 2 January 2016 at 10:59, Paul Stoffregen <pa...@pjrc.com> wrote:
> As nearly as I can tell, EthernetServer can only implement protocols like
> HTTP where the conversation begins by the client sending data first.

> [snip]

> Have I just overlooked or misunderstood something? Or does Arduino's API
> lack any way to implement such servers?

AFAIK you're not overlooking something. I discovered this, errr, some
time ago :), when implementing an Arduino VNC server:

* <http://code.rancidbacon.com/LearningAboutArduinoVNC>
* <http://forum.arduino.cc/index.php?topic=7376.0>

From my notes at the time: "...both Arduino and Processing define the
Server.available method as meaning 'Gets a client that is connected to
the server _and has data available for reading_.' So there's no way to
retrieve a client that has connected but not yet sent any data...".

The first link has the quick patch I used and to my surprise
apparently I got the TODO as far creating an issue and patch:
<https://code.google.com/p/arduino/issues/detail?id=186> /
https://github.com/arduino/Arduino/issues/186 :)

--Philip;

[Adrian McEwen]

Agreed. We spent some time trying to fix this when I implemented the
Server base class (and so EthernetServer, etc) but got caught between
wanting to fix the broken API and also keep compatibility with
Processing's API. I ran out of steam when it looked had to convince
Processing to fix it too, sorry :-(

There was a thread about it on the old developers mailing list about
it. It had the subject "The Server API for accepting new connections",
but I can't find the archive online to point directly to it. I'm not
sure I've got all of it myself, but have some of it. There's some good
discussion in there, which I can try to reconstruct if there isn't a
copy available online somewhere.

My initial suggestion was:

"And while we're talking about changes to Server::available, it would be
nice if we could fix the problem that Philip flagged a while back (and
which I hadn't spotted until this evening)
<http://code.google.com/p/arduino/issues/detail?id=186>. Basically
Server::available only returns a Client if there's data waiting from it,
rather than if it's connected.

I haven't done enough with the Server class to realise that - I'd just
assumed that Server::available returned the next "available" connection,
rather than one with data. I don't think the current examples (and so
most likely almost all existing code?) assume that there's data
available, so we could just change Server::available so that it returns
connected connections and doesn't care about any data on them. Anyone
have any objections to that? We could change the name (to accept?) if
people think it's better to make the shift in behaviour explicit."

I got as far as implementing it to try out - see
<https://github.com/amcewen/Arduino/commit/8513e4399e8b6e3efcc1dfdb7a3067d897c20cdb>.
I expect it'd port in without any real problems...

Cheers,

Adrian.

[Paul Stoffregen]

On 01/02/2016 04:04 AM, Adrian McEwen wrote:
> .... so we could just change Server::available so that it returns

> connected connections and doesn't care about any data on them. Anyone
> have any objections to that?

Always returning all the connected clients is inefficient, and it will
scale poorly as we move to more capable networking hardware.

> We could change the name (to accept?) if people think it's better to
> make the shift in behaviour explicit."

I believe we should add Server.accept() to the API. It would return any
newly connected client, only once, regardless of whether any data has
arrived. After being returned by Server.accept(), the client would no
longer be managed by that Server object. Future calls to
Server.available() would not return that client, and Server.write()
would not transmit data to it.

If any change is made to the existing API, I believe EthernetServer
should manage only a single client at a time. Multiple-client support
in Server.available() and Server.write() is a difficult-to-use feature.
The ChatServer examples use this to receive data from any client and
broadcast it to all others. But outside of that narrow application,
everyone seems to have built one-client-at-a-time programs. Even for
the chat, the AdvancedChatServer example has to maintain an array of
clients and compare the return from Server.available() against the
entire list (for every byte arriving), just to detect the difference
between a newly-connected client and new data.

Indeed the Server object is very easy and convenient for single-client
designs, and virtually all existing projects are single-client-at-a-time
approach. Even Webduino uses a single-client design.
https://github.com/sirleech/Webduino/

Multi-client support within Server is a pitfall (and possibly a security
vulnerability) just waiting to happen. Another client can connect
(strike without warning) at any time and begin feeding you its data
instead/intermingled. All clients receive a copy of whatever you're
writing to the Server object. I hope it's easy to see how this could be
terribly insecure when used in the easiest way (as the examples
demonstrate), especially in an IoT future where people use Arduino to
make their connected devices!

[Andrew Kroll]

Yet another reason I use my own IP stack... ;-)

[Matthew Ford]

Check out the ESP8266 arduino package
in WifiServer.cpp

WiFiClient WiFiServer::available(byte* status) {
if (_unclaimed) {
WiFiClient result(_unclaimed);
_unclaimed = _unclaimed->next();
DEBUGV("WS:av\r\n");
return result;
}

optimistic_yield(1000);
return WiFiClient();
}

underlying tcp puts new connections on a list and available just returns
the next unclaimed.

[Adrian McEwen]

My suggestion was poorly worded. I wasn't proposing that available
return /all/ connected clients, just that it would only return a new
connection (rather than connection and/or data, as at present). So
basically, yes, we're in complete agreement :-D

Adrian.

[Paul Stoffregen]

On 01/03/2016 09:35 AM, Adrian McEwen wrote:
> My suggestion was poorly worded. I wasn't proposing that available
> return /all/ connected clients, just that it would only return a new
> connection (rather than connection and/or data, as at present). So
> basically, yes, we're in complete agreement :-D

I actually have mixed feelings about. I would have been compete
agreement in 2011 when this API was published.

But since then, people have used the Server.available API to detect when
data is ready, not just the arrival of new connections. For example:

https://github.com/ovidiucp/TinyWebServer/blob/master/TinyWebServer.cpp#L232

Changing Server.available now is going to break some well established
programs. I'm not sure how I feel about that....

[Tom Igoe]

As someone who maintains a bunch of legacy code that relies on Server.available() in the way Paul describes, I’m also not too happy about the idea of breaking it. However, I do see the need for an event when a client connects, so the server can send an initial message. 

A better solution might be to do a variation on Processing’s serverEvent() (https://processing.org/reference/libraries/net/serverEvent_.html). Since we don’t currently have events in the core, we’d need something that could be polled for, but I’m sure we could come up with something workable: Perhaps Server.clients() could be an incremental count of clients, for example?

Tom Igoe

[Adrian McEwen]

You do realise your currently legacy code is broken then, right? ;-)  Admittedly you might not notice until you get weird, hard-to-track-down bugs when >1 client connects at the same time...

If we're going to poll for something, my vote would be to add an accept() call instead.  That won't confuse people expecting serverEvent to just happen, and would mirror the sockets usage everywhere else.

Cheers,

Adrian.

[Andrew Kroll]

Not sure on how this library does this sort of thing, but in my IP stack I do things a touch different than most.
accept() is blocking:
https://github.com/xxxajk/ajkstack/blob/master/socket.c#L902
but I can actually test if any inbound connection is waiting first:
https://github.com/xxxajk/ajkstack/blob/master/socket.c#L865

...which returns the socket fd number to operate on, thus allowing multiple connections.

As far as worries with running out of RAM, you just do two things:

1: set the listen backlog to something sane, like 3. waiting backlog connections won't eat much RAM if things are written correctly.

2: service only a few connections at a time.

That said, I do need to finish different hardware support besides SLIP via serial, but in order for me to do that, I kind of need to find time for that. Feel free to look at my implementation, and barrow/steal any bits you deem useful. Oh and yes, this IP stack really does run on 1970's z80 CP/M machines. which have to share code and data, which makes an AVR feel roomy....

Visit my github for awesome Arduino code @ https://github.com/xxxajk

[Tom Igoe]

Point is, .available() is used by Stream in many different implementations and always gives a byte count of the data (even if “broken” here). To reuse that name for something else seems wrong to me. So following your logic, then, would accept() and return a Client, from which you could then read .available()?

Tom Igoe

[Andrew Kroll]

My implementation does not have any sort of way to see if there is data available, instead read is non-blocking.

If you think about this pragmatically, testing if there is data is actually a waste of time, when a non-blocking variant could return zero octets and you just skip any processing anyway.

[Adrian McEwen]

Yes.

And fair enough on not reusing available().  I'd only suggested that initially as (a) knowing what happens behind the scenes it initially never occurred to me that you'd ever want to return data from a Server :-D and (b) I didn't know if anyone used the returns-data version beyond the ChatServer example (I checked your Making Things Talk examples at the time and they were okay).

Legacy aside - i.e. I'm /not/ suggesting we change Server beyond adding accept(), and ideally updating the documentation to recommend against using Server.available() - I don't think it makes sense for Server to derive from Stream - although it has a socket and so looks a bit like a Client, it never makes sense to write to it, and we end up with things like available which intermingles arriving data.  Anyway.

Cheers,

Adrian.

[Tom Igoe]

On Jan 4, 2016, at 7:26 AM, Adrian McEwen <adr...@mcqn.net> wrote:

Yes.

Well, that was easy! Now we just need an implementation. 

And fair enough on not reusing available().  I'd only suggested that initially as (a) knowing what happens behind the scenes it initially never occurred to me that you'd ever want to return data from a Server :-D and (b) I didn't know if anyone used the returns-data version beyond the ChatServer example (I checked your Making Things Talk examples at the time and they were okay).

It has always bothered me a bit anyway, to be honest, and I’ve generally advised against writing servers on the Arduino if you can avoid it, because it’s not a good use of the processor. If it’s on the net, there is a more capable processor for a server that it can talk to. So we’re in the same mindset. Thanks for checking those, though; as it turns out, some maintenance on that is on my agenda this month anyway.

Legacy aside - i.e. I'm /not/ suggesting we change Server beyond adding accept(), and ideally updating the documentation to recommend against using Server.available() - I don't think it makes sense for Server to derive from Stream - although it has a socket and so looks a bit like a Client, it never makes sense to write to it, and we end up with things like available which intermingles arriving data.  Anyway.

Those sound like reasonable suggestions to me, and a good start to a solution. We’d need to provide access to the client list in a not-ugly way, so you can broadcast, and we’d need to know which client spoke most recently, but maybe Processing’s clientEvent() and serverEvent() can provide models.

t.

[Paul Stoffregen]

It sounds like we're all in agreement for Server.accept().  I'd be happy to put this together and submit a pull request.  There's already an accept function in the code that's very close.  I do want to take a little time to rewrite the chat server examples to make use of Server.accept().

Tom's sentiment "generally advised against writing servers on the Arduino if you can avoid it, because it’s not a good use of the processor" makes a lot of sense on Arduino Uno with only 2K RAM and the W5100 ethernet chip.

Modern ARM Cortex-M microcontrollers are an entirely different matter.  Many of them already have very capable DMA-based ethernet hardware.  In response to the "Internet Of Things" craze, the semiconductor manufacturers are all developing tightly integrated wireless networking.

We need a good server API for the not-too-distant future when these networking interfaces become as common in microcontrollers as USB.

[John Plocher]

Paul,

Can you help me understand how this change would break existing code?

Right now, the existing code only works with data-available clients, like HTTP. 

It doesn't work with wait-for-welcome clients.

Any sketch that uses available() is 

  A) already broken in that "wait-for-welcome" clients don't work, and 

  B) doesn't care, because they will never see such a client due to the protocol(s) they are implementing.

Because of "B", won't this change be invisible and benign to those existing use cases?

Having said all that, why invent yet another socket-like API abstraction from scratch (with all the bugs involved...) instead of reusing a design pattern that is already proven elsewhere?

  -John